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

[Elasticsearch] Way forward for ES integration? #179

Closed
dkarlovi opened this issue Oct 17, 2017 · 9 comments
Closed

[Elasticsearch] Way forward for ES integration? #179

dkarlovi opened this issue Oct 17, 2017 · 9 comments
Labels

Comments

@dkarlovi
Copy link
Contributor

Q A
Bug report? no
Feature request? no
BC Break report? no
RFC? yes

So, looking at search-elasticsearch, it seems it some ways off from general availability. As my target use-case is using Elasticsearch with API Platform (as noted in api-platform/core#991) and your elaborate search solution seems like a good fit, what would be a good way for me to go forward?

I'd fork the repo to add some of the required functionality myself, but seeing all code is currently in a WIP branch, don't want to branch from a dead end.

Also, do you have any docs with planned features?

@sstok sstok added the RFC label Oct 17, 2017
@sstok
Copy link
Member

sstok commented Oct 17, 2017

I pushed some bug fixes and other changes.

If you can help with the implementation that would be great 👍 especially the functional testing.
The Doctrine DBAL/ORM condition-processors use mocked-up e-commerce system to ensure the most common senario's work. If you can help with setting this up for ElasticSearch then writing the implementation will be easier as we can know for sure it's working as expected.

@dkarlovi
Copy link
Contributor Author

I'm guessing you mean rollerworks/search-elasticsearch@738bb8c?

The Doctrine DBAL/ORM condition-processors use mocked-up e-commerce system to ensure the most common senario's work.

OK, seems fair, I'll look into it right away.

@dkarlovi
Copy link
Contributor Author

@sstok could you submit to Packagist? That should allow me to install my fork for testing.

@dkarlovi
Copy link
Contributor Author

Also, I see you've removed Elastica as a dependency, can you clarify why?

@sstok
Copy link
Member

sstok commented Oct 18, 2017

I removed it because it's not really needed to make this integration work.
The generator can produce a Query using array's only (which saves allot of memory in this case).

But don't worry an adapter for Elastica is definitely planned ;)

@dkarlovi
Copy link
Contributor Author

OK, I guess it's OK to include it as a dev requirement?

sstok added a commit to rollerworks/search-elasticsearch that referenced this issue Oct 24, 2017
This PR was merged into the implementation branch.

Discussion
----------

As suggested in rollerworks/search#179

Commits
-------

68b72b7 infra: switch the build process to a Docker-based one (including Travis)
8e45271 infra: tweak PHP-CS-Fixer, add PHPStan config
2e857c2 tests: initial functional tests
13c97be WIP
f8eb31a Range ID lookup
dbaad39 Date lookup
76dfbdb WIP nested query
731dd0d WIP with conversions (broken currently)
beffb03 WIP with conversions
0a8c01d WIP with conversions
c0b53c2 WIP MoneyType
1698882 WIP range support
760c48e WIP range support
21445cc WIP value conversion <-> query conversion cleanup
3b94110 WIP rename QueryConversionHints to QueryPreparationHints
dfdb919 WIP PHPStan fixes
5c9a6f7 WIP nested queries support
@dkarlovi
Copy link
Contributor Author

dkarlovi commented Oct 25, 2017

After merging the query generator, there are a couple of things needed here still, please correct me if I'm wrong here somewhere:

  1. ElasticsearchFactory in search-elasticsearch, this one seems simple. Done in Add a Generator interface, CachedGenerator + Factory search-elasticsearch#3
  2. search-elasticsearch support in RollerworksSearchBundle, both "regular" and api-platform sections, progress in WIP: Add Elasticsearch support RollerworksSearchBundle#37
  3. search-elasticsearch support in search-api-platform, much like the Doctrine stuff, progress in WIP: Add Elasticsearch support search-api-platform#8

For 2):

  1. we'd need to figure out how to build the mapping for the index/type. For now, I think it would be OK to rely on the user to do it on his/her own if the bundle suggests FOS Elastica Bundle (this would probably make sense because, as a advanced use case, you probably want control over your mappings). Delegate this to FOS Elastica bundle, we're just using it.
  2. Also, if you have both Doctrine and Elasticsearch enabled, it should do a query on Elastic and then fetch the records (by ID) from Doctrine. Forgot FOS Elastica Bundle already does that, one more reason to depend on it. We're actually modifying the query from an Extension currently, it should be OK for now.

@sstok
Copy link
Member

sstok commented Oct 25, 2017

Agreed. RollerworksSearch is about the searching of records, index building and mapping is better suited by there integrations. This is also the main goal of this system, to generalize the searching in various storage systems, as this is the most complex part of any search system.

So for the API-Platform I guess we only need a CollectionDataProvider implementation that performs the Elasticsearch query and returns the result for the API-Platorm serializer part.
And a Paginator of course.

There is noting needed for writing on our part?

@sstok
Copy link
Member

sstok commented Oct 25, 2017

FYI, I looked into merging all repositories into a single one (while preserving the history as much as possible). This is possible but will require some work 💯 😄 there are total of 10 repositories (well 9 actually as pomm is empty atm).

So 🔜 we will have a mono-rep to speed-up development.

sstok added a commit that referenced this issue Nov 17, 2017
… Platform (dkarlovi)

This PR was merged into the 2.0-dev branch.

Discussion
----------

Porting rollerworks/search-api-platform#8, rollerworks/RollerworksSearchBundle#37 to new mono-repo.

Closes #179.

Commits
-------

00ee5b3 Integrate existing Elasticsearch support to Symfony, API Platform
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

2 participants