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

ES 5.1? #51

Closed
ellisio opened this issue Jan 20, 2017 · 13 comments
Closed

ES 5.1? #51

ellisio opened this issue Jan 20, 2017 · 13 comments
Labels

Comments

@ellisio
Copy link
Contributor

ellisio commented Jan 20, 2017

Seeing as how this is still under active development, have you thought about making sure this is built against the latest ES 5.1?

Upgrading composer.json to the following:

"elasticsearch/elasticsearch": "~5.1",
"ongr/elasticsearch-dsl": "dev-master"

Then running your unit tests results in a full pass. I know you probably don't want dev-master as dependency right now. But being under active development can't hurt to do so seeing as how ongr/elasticsearch-dsl is actively working on releasing their 5.x support.

$ vendor/bin/phpunit
PHPUnit 4.8.31 by Sebastian Bergmann and contributors.

Runtime:	PHP 7.0.14 with Xdebug 2.5.0
Configuration:	/Users/andrew/Developer/GitHub/ellisio/plastic/phpunit.xml

...............................................................  63 / 141 ( 44%)
............................................................... 126 / 141 ( 89%)
...............

Time: 2.72 seconds, Memory: 18.00MB

OK (141 tests, 246 assertions)

Generating code coverage report in Clover XML format ... done

Generating code coverage report in HTML format ... done
@sleimanx2
Copy link
Owner

perfect added a develop branch for ES 5 support

@ellisio
Copy link
Contributor Author

ellisio commented Jan 25, 2017

I think this needs to be reopened and discussed a little more.

Right now you're wanting to include support for Laravel 5.2.*|5.3.*. Well, 5.4.* is now a thing.

Also, the underlying library of elasticsearch/elasticsearch is going to require PHP ^7.0 as they require ext-json >= 1.3.7. PHP 5.6.x ships with ext-json 1.2.1.

So if we want to support the elasticsearch/elasticsearch 5.x the minimum PHP version Plastic will need to support is 7.0. This cuts out all current Laravel versions as the framework only requires 5.6.4+.

Laravel 5.5 will require PHP 7.0+. So until Laravel requires 7.0 I would halt development and support on ES 5.

Once Laravel 5.5 is released we can change the composer.json to the following:

  "require": {
    "php": ">=7.0",
    "elasticsearch/elasticsearch": "~5.1",
    "ongr/elasticsearch-dsl": "~5.0"
    "illuminate/container": "5.5.*",
    "illuminate/contracts": "5.5.*",
    "illuminate/console": "5.5.*",
    "illuminate/pagination": "5.5.*",
    "illuminate/support": "5.5.*",
    "illuminate/database": "5.5.*",
  },

@jamesgraham
Copy link
Contributor

@ellisio I understand where you come from but what solution can we arrive at for people running PHP7 and wanting ES 5? Should we have a new major version of this library for these latest versions?

@ellisio
Copy link
Contributor Author

ellisio commented Feb 3, 2017

So after some discussion with the elasticsearch/elasticsearch author, he was willing to add a new library to the class to help out our cause of keeping support for 5.x. Take a look:

elastic/elasticsearch-php#534 (comment)

@jamesgraham
Copy link
Contributor

@ellisio how should we use this now?

@jamesgraham
Copy link
Contributor

@sleimanx2 @ellisio The ES library has been updated to 5.1.3 to support this - what are the next steps?

@sleimanx2 sleimanx2 reopened this Feb 17, 2017
@dreferreira
Copy link

@ellisio @sleimanx2 any further update on how to get this to be compatible with ES 5.X?? I'm currently working on a large project involving laravel 5.4 with PHP 7.0.2 (Which is required for some of the processing within laravel) and would like to require this package with the latest ES.

@dominicmomentum
Copy link

@ellisio @sleimanx2 I'm also very interested having this package compatible with ES 5.X and PHP 7. Could we create a separate branch for that until Laravel 5.5 in July?

@sleimanx2
Copy link
Owner

the issue is that the test are not green anymore due to the following update

Since Elasticsearch 5.0 the support for top level filters was dropped. The same functionality is now supported via BoolQuery

I'll work on fixing the issue soon.

@sleimanx2 sleimanx2 reopened this Apr 5, 2017
@sleimanx2 sleimanx2 added the next label Apr 5, 2017
@hipsterjazzbo
Copy link
Contributor

@sleimanx2 What needs doing on this? Is the work on the develop branch current, or is it out of date? I note that it is currently 32 commits behind master.

@hoersten
Copy link

@sleimanx2 Any updates on this? We'd like to use it on our Laravel 5.4, ES 5.4 and PHP 7 site.

@jamesgraham
Copy link
Contributor

Hey @sleimanx2 is there anything the community can do to help move this?

@sleimanx2
Copy link
Owner

Please check v0.5.0 🥂

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

No branches or pull requests

7 participants