Skip to content
This repository was archived by the owner on Feb 5, 2024. It is now read-only.

Persist and remove methods for Manager #155

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

tomcyr
Copy link
Contributor

@tomcyr tomcyr commented Jan 25, 2013

This is my implementation of persist and remove methods without Unity of Work pattern. I know that Unity of Work exists in all Doctrine Mappers (I'm already using ORM and Mongo ODM Mapper) but in case Orient DB where every method is called by REST API i think that's no sense of using Unity of Work. Maybe you have other ideas or proposal?

@odino
Copy link
Member

odino commented Jan 27, 2013

Hi @tomcyr,

even though we - for now - have only written the HTTP binding, most likely we will need to also support the binary protocol implementation, which is why we created the binding interface. I remember I also did a few benchmark with the other OrientDB PHP driver (but it was damn slow), so I wouldnt force users to use the HTTP protocol, if they are not willing to.

In any case, this PR is very useful as we can move some methods from the manager to the specific bindings and then let them handle persistence (the HTTP one will probably make a call for every query, the binary one will use transactions or so on). @nrk any thoughts?

Also, what about @lvca's comment on HTTP transactions? Luca, is there a way to send a bunch of queries in one shot trough the HTTP protocol?

@lvca
Copy link

lvca commented Jan 27, 2013

Hi @odino, probably now the PHP binary driver fixed the problem about performance? About Transactions it's a simple task to do, probably I could do it in the next week.

@odino
Copy link
Member

odino commented Jan 28, 2013

@lvca the problem itself is how you need to parse stuff, char by char, as in PHP its expensive. A C native driver would help a lot, as we can build a php extension on top of it.

Transactions: yes BLEASE! :)

@lvca
Copy link

lvca commented Jan 28, 2013

Hi,
probably it wouldn't be so bad using the C driver from PHP but I guess it's
not so easy for everyone.

About tx we've: orientechnologies/orientdb#90

Lvc@

On 28 January 2013 13:16, Alessandro Nadalin [email protected]:

@lvca https://github.com/lvca the problem itself is how you need to
parse stuff, char by char, as in PHP its expensive. A C native driver would
help a lot, as we can build a php extension on top of it.

Transactions: yes BLEASE! :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/155#issuecomment-12779249.

@nrk
Copy link
Member

nrk commented Jan 28, 2013

@odino I don't have quite a clear idea on the matter yet, I prefer to wait and see how transactions are going to be supported by OrientDB in its HTTP REST interface to make a comparison with how they are supported by the binary protocol interface before hazarding an answer :-)

@nrk
Copy link
Member

nrk commented Feb 1, 2013

Some additions in this commit such as the use of putDocument() in the manager highlight what's probably a flaw in our current abstraction that we should address now that there's some early work on persistence: the manager class currently accepts a Doctrine\OrientDB\Binding\BindingInterface in its constructor, which in turn defines only a BindingInterface::execute() method. As a temporary fix we should change the constructor signature to use Doctrine\OrientDB\Binding\HttpBindingInterface just to play safe.

Next thing is to think about moving more methods from Doctrine\OrientDB\Binding\HttpBindingInterface to Doctrine\OrientDB\Binding\BindingInterface but on the other hand the naming of certain methods do not make sense in a broader abstraction since methods like postDocument() or putDocument() do not really make sense outside the realm of the HTTP REST API, while names like createDocument() and updateDocument() would.

I don't have a clear idea in mind right now, it's just a thought thrown here to keep track of it and spin the discussion to a broader level.

@odino
Copy link
Member

odino commented Feb 3, 2013

@nrk totally up for that

@tomcyr I would still keep this methods outside of the Manager itself. orientdb v1.4.0 implements the multi-operations for HTTP protocol, so it makes sense to base the persistence on top of that

@tomcyr
Copy link
Contributor Author

tomcyr commented Feb 28, 2013

OK guys, I created OrientDbBundle and I implementented persistance methods in my Service class outside Manager so in future this methods can be used from Manager:)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants