Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement elasticsearch-php as foundation of Elastica #944

Open
ruflin opened this issue Sep 30, 2015 · 19 comments
Open

Implement elasticsearch-php as foundation of Elastica #944

ruflin opened this issue Sep 30, 2015 · 19 comments

Comments

@ruflin
Copy link
Owner

ruflin commented Sep 30, 2015

Elastica is a high level elasticsearch client, elasticsearch-php is a low level elasticsearch client. What both have in common are the low level implementations of connector, transport etc. Instead of trying both clients up to date with the newest improvements on these levels I suggest to take the elasticsearch-php client as the foundation of Elastica for the connection part to elasticsearch. This will bring together efforts to implement things like Guzzle and connection strategies.

As an approach for implementation I suggest to extend \Elastica\Client by \Elasticsearch\Client and make the \Elastic\Client use connection and transport form \Elasticsearch\Client. If possible backward compatibility for all request should not break.

The above will in addition allow every user of Elastica to use all low level functionality of the extended client in addition.

@xkidro
Copy link

xkidro commented Oct 16, 2015

+1 for this, I currently use the elasticsearch-php sdk but found that I need to implement high level functions myself :(

@ruflin
Copy link
Owner Author

ruflin commented Oct 16, 2015

@xkidro Good to hear. First I was thinking about introducing this with the es 2.0, but there are still quite a few open changes, so perhaps I will even prioritise this one higher. If you have some to help with this migration, that would be cool.

@CrAzE124
Copy link
Contributor

+1 definitely, I haven't been on this repo for a while, and was just wondering whether this was possible. How can I help on this?

@ruflin
Copy link
Owner Author

ruflin commented Oct 28, 2015

@CrAzE124 My plan to implement this was as following, but because of 2.0 I didn't have the time yet to try it out.

  • Elastica\Client should extend php-elasticsearch::Client
  • At first, the only method that uses php-elasticsearch::Client is Elastica\Client::request(...).

The above would mean, php-elasticsearch has to be added to the composer file and the request method has to be adjusted so that it calls the php-elasticsearch client. At first, it doesn't even have to extend Elastica\Client.

The best help would be to try this out an open a pull request with these changes so we can discuss it on actual code.

@merk
Copy link
Collaborator

merk commented Mar 4, 2016

@ruflin I've been hitting issues with FOSEB which aren't worth fixing and prompt me to start over. Where are you at with this change? Is it still on the radar?

I dont really want to start from scratch on a dependency of Elastica if Elastica is going to move to become a value add on top of the official client, I'd prefer to support both from the get go.

@ruflin
Copy link
Owner Author

ruflin commented Mar 4, 2016

@merk This is definitively still on the radar. Unfortunately it seems like @CrAzE124 and @ewgRa both didn't find the time yet to progress on this one. Let me see if I find a way to push this change in faster.

Interesting to hear about FOSEB. Is it something that changed in Elastica or on the elasticsearch side? Would be interesting to hear more.

@merk
Copy link
Collaborator

merk commented Mar 4, 2016

The biggest pain point right now is the hard BC breaks from 2.x to 3.x of Elastica - given you move your required ES version with each release, we cant maintain support for ES1.7 (or any other release version) without maintaining multiple branches - coupled with a low test coverage and removal of features in Elastica 3 its unlikely or very hard to support both in one code branch.

Fundamentally, I'm not willing to break support for older versions unless there is a very real and important reason to, and since FOSEB is only using the most basic features of Elastica, it doesnt make sense to track your latest versions only.

@merk
Copy link
Collaborator

merk commented Mar 4, 2016

And I think the biggest pain point moving forward is managing index and transformation metadata. There is no generic method for defining this information, and the way we're handling it now is fragile and prone to breakage. I still havent been able to work out a decent enough solution. #810

@ruflin
Copy link
Owner Author

ruflin commented Mar 7, 2016

@merk It would be great if we could discuss the above points in an audio / video chat to go into more details. Let me know if you would be interested in having this direct conversation. I just sent you an email with my email :-) In case someone else would like to join the conversation, drop a message here.

@ewgRa
Copy link
Contributor

ewgRa commented Mar 7, 2016

I haven't time for this right now, but still interested. Probably at the end of March I will have time for try this.

@merk why you want to support all versions at one package? What you want to avoid?

@ruflin
Copy link
Owner Author

ruflin commented Mar 7, 2016

@ewgRa In case we have a chat, I will for sure put a summary / conclusion here.

@merk
Copy link
Collaborator

merk commented Mar 7, 2016

@ruflin Sounds good. I've got a few things to finish off over the next day but will ping you when I get a chance to free up some time.

@ewgRa Not all versions, I'm just against BC breaks for the sake of breaks (and version incompatibilities) and want to see Elastica become easier to integrate.

@ruflin
Copy link
Owner Author

ruflin commented Mar 7, 2016

@merk 👍

@erichard
Copy link

erichard commented Mar 9, 2017

Hello there, I'm using the official elasticsearch-php SDK but I lack elastica feature like fluent builder.
Are you still working on this ? What is missing to move on ?

@ruflin
Copy link
Owner Author

ruflin commented Mar 11, 2017

@erichard That is the definitively still on the to do list. The main thing that is missing for the move is the time to do it :-) We had a few PR's in the past but they were never fully completed. So if someone else has time to put some brain power into how to do this best, that would be really helpful.

ruflin pushed a commit that referenced this issue Mar 21, 2017
There is old story about implement elasticsearch-php as foundation of Elastica #944

Also as some proposals, that stuck in "client-transport-connection" layer problem, that can't be easy solved.

But we can already use official client benefits without waiting solution for transport layer. I suggest make first step not from solving transport layer, but from starting usage Endpoints.

This PR shows how official client can be used right now, for example for Stats endpoint.
Also as in ClientTest example how Search endpoint can be used.
@ruflin
Copy link
Owner Author

ruflin commented Mar 21, 2017

We now have Elastica depending on the official client: #1275 It is only a first step to make the functionality available but this will now allow us to iterate on top of it.

mhernik pushed a commit to mhernik/Elastica that referenced this issue Jul 24, 2017
There is old story about implement elasticsearch-php as foundation of Elastica ruflin#944

Also as some proposals, that stuck in "client-transport-connection" layer problem, that can't be easy solved.

But we can already use official client benefits without waiting solution for transport layer. I suggest make first step not from solving transport layer, but from starting usage Endpoints.

This PR shows how official client can be used right now, for example for Stats endpoint.
Also as in ClientTest example how Search endpoint can be used.
@akalongman
Copy link

Someone is working on this?

@ewgRa
Copy link
Contributor

ewgRa commented Apr 6, 2018

Me not and I haven't any plans to work on it.

There was good step forward in this direction #1277 .

Next step will be harder and probably require big brake backward compatibility, but it can be a final step for this issue. I personally would be happy to see PR for next step :)

@ruflin
Copy link
Owner Author

ruflin commented Apr 10, 2018

@akalongman As @ewgRa the foundation is there. Now the questions are which are the big next steps and how to get there in the least breaking way.

One thing I would really like to see is that long term the Connection and Transport package in Elastic compeltely disappear and it fully relies on the official client.

@akalongman If you have time to work on it and think more about it I'm more then happy to discuss ideas and push this forward with you.

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

No branches or pull requests

7 participants