datastore: Transaction sends non-transactional requests

[ryanseys]

Confusing that a Transaction object can send non-transaction requests. It would be better if this logic was abstracted into something like a "APIRequest" or something.

Instead of instantiating Dataset with a transaction object, it should just be a API request creator object.

Maybe I'm being too picky.

[stephenplusplus]

👍 it is a bit confusing at first. Maybe:

function ApiRequest() {}
ApiRequest.prototype.delete
ApiRequest.prototype.get
ApiRequest.prototype.mapQuery
ApiRequest.prototype.makeReq
ApiRequest.prototype.runQuery
ApiRequest.prototype.save

function Transaction() {}
Transaction.prototype.begin
Transaction.prototype.commit
Transaction.prototype.finalize
Transaction.prototype.rollback

function Dataset() {}
util.extend(Dataset, ApiRequest);

We would just need to figure out how to get jsdoc to handle this.

[ryanseys]

Why not have Dataset just create an ApiRequest object and return that? We can use this to create requests for all the APIs in this library (much like is done in google-api-nodejs-client. Transaction can just be designed to make transactional ApiRequests.

[stephenplusplus]

Why not have Dataset just create an ApiRequest object and return that?

👍 assuming you mean Dataset methods (.get, .whatever) return ApiRequest objects.

Can you put together some rough code so I can better visualize how dataset/transaction would work with the new ApiRequest?

[ryanseys]

assuming you mean Dataset methods (.get, .whatever) return ApiRequest objects.

Yes.

Can you put together some rough code

Yes. I'll try and get a rough implementation out this weekend.

[ryanseys]

Are we still doing this? I held back on implementing this because I wasn't sure what others thought. @silvolu @rakyll

[rakyll]

I don't like the fact that non-transactional calls are abstracted as method's of a pseudo transaction either. @ryanseys, could you propose an API with ApiRequest objects? I don't fully understand how it will look from the user's perspective.

[ryanseys]

Uhh, the API should not change as far as the user is concerned. More just refactoring. Separation of responsibility.

[rakyll]

SGTM.