Adding execution to request with some utilities to create MongoContro…

lightblue-platform / lightblue-client

GNU General Public License v3.0

5 stars 24 forks source link

Adding execution to request with some utilities to create MongoContro… #297

Closed paterczm closed 8 years ago

paterczm commented 8 years ago

…ller options

coveralls commented 8 years ago

Coverage decreased (-0.02%) to 67.875% when pulling c0b16e335a0fbc1b5f8cac047342cb5d0d9acc91 on execution into 61efff01b033fd38e68fa31bf108be23a7dfcbc8 on master.

dcrissman commented 8 years ago

Is Execution needed on GenerateRequest or LiteralDataRequest?

dcrissman commented 8 years ago

This might be a follow up change, but it seems like we would also want a way to specify these options in the lightblue-client.properties file. Maybe this could be the default for all requests, but it can be overridden by setting the Execution directly?

jewzaam commented 8 years ago

@dcrissman +1 on clients being able to set a default. Agree this is a follow up. Could you log an issue if you haven't?

paterczm commented 8 years ago

@dcrissman

Is Execution needed on GenerateRequest or LiteralDataRequest?

The point of LiteralDataRequest is that you need to provide full request body yourself, including execution.

I guess writeConcern would make sense for GenerateRequest, but value generator implementation in lightblue-mongo does not support it. Same with locking functionality. For now, execution is only supported for standard crud operations.

dcrissman commented 8 years ago

Is Execution needed on GenerateRequest or LiteralDataRequest?

The point of LiteralDataRequest is that you need to provide full request body yourself, including execution.

I guess writeConcern would make sense for GenerateRequest, but value generator implementation in lightblue-mongo does not support it. Same with locking functionality. For now, execution is only supported for standard crud operations.

Sorry, I was not arguing for them to have the fields. What I meant is that Generated/LiteralRequest both inherit from AbstractLightblueDataRequest, so by adding the getter/setter for Execution to AbstractLightblueDataRequest they both get them. So I meant to ask do we want these Requests to also have Execution?

coveralls commented 8 years ago

Coverage increased (+0.4%) to 68.287% when pulling b11f4e655f1bbae804db761d1de7b561e722ec96 on execution into 61efff01b033fd38e68fa31bf108be23a7dfcbc8 on master.

paterczm commented 8 years ago

@dcrissman, I see what you mean now. You're right. I added one more abstract layer (AbstractLightblueDataExecutionRequest) so that GenerateRequest and LiteralDataRequest do not provide execution api. I don't like having so many abstract layers, but an alternative is some redundancy in crud requests, which I also don't like.

coveralls commented 8 years ago

Coverage increased (+0.5%) to 68.356% when pulling af8b96a03eee9c87b4e44edc0fe1a2f9536b2ddf on execution into 61efff01b033fd38e68fa31bf108be23a7dfcbc8 on master.

paterczm commented 8 years ago

All feedback is incorporated now, unless otherwise noted in the comments.

coveralls commented 8 years ago

Coverage increased (+0.4%) to 68.339% when pulling 1b9d7c519a0c3785d89243283c22077839760428 on execution into 61efff01b033fd38e68fa31bf108be23a7dfcbc8 on master.

coveralls commented 8 years ago

Coverage increased (+0.5%) to 68.356% when pulling 48a5550145d849484434eba52bdc9afea8b293b4 on execution into 61efff01b033fd38e68fa31bf108be23a7dfcbc8 on master.