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

Support for Elastic 7.x #1692

Closed
1 of 12 tasks
pkarw opened this issue Sep 4, 2018 · 9 comments
Closed
1 of 12 tasks

Support for Elastic 7.x #1692

pkarw opened this issue Sep 4, 2018 · 9 comments
Labels
feature request Requests for new features. Please be as specific as possible and provide proposal API if it you can P2: Important Priority mark - still high ;)
Milestone

Comments

@pkarw
Copy link
Collaborator

pkarw commented Sep 4, 2018

  • I'm submitting a ...

    • bug report
    • feature request
    • support request => Please do not submit support request here, see note at the top of this template.
  • What is the current behavior?

Currently we’re storing multiple document types (product, attribute, categories, taxrules) in single index (vue_storefront_catalog by default). ES 6.x is no longer supporting this setup and requires us to have:

  • single index per entity type (A)
  • custom type field (B)

https://www.elastic.co/guide/en/elasticsearch/reference/6.x/removal-of-types.html

I believe that we should follow the A (looking forward to Your feedback!) using the following naming convention for indexes:

  • vue_storefront_catalog_product
  • vue_storefront_catalog_category
  • ...

We should avoid as many changes required on the users as we could.

  1. Changes in the config
    The index name should be changed just before reaching the ES - ** without requiring the config changes** (eg. we have vue_storefront_catalog in the config so we can just add the entityType to indexName in here: https://github.com/DivanteLtd/vue-storefront-api/blob/e0aa58460f6a986e2527f364946fc751620d7428/src/api/catalog.js#L55)

  2. Automatic migration
    We can base on apiVersion ( https://github.com/DivanteLtd/vue-storefront-api/blob/e0aa58460f6a986e2527f364946fc751620d7428/config/default.json#L33) and check - if there is the 6.x selected we should run the automatic migration in vue-storefront-api + placing a lock file like .es.migrated - to let the script know no further migration is required.

It can be a separate migration script or just another migration (then no lock file is required) -> https://github.com/DivanteLtd/vue-storefront-api/tree/master/migrations

  1. Front-end
    This change should be TRANSPARENT to the frontend VS - with no changes required (both config, and code)

To do so we must refactor:

  • vue-storefront-api/src/scripts/db.js
  • vue-storefront-api/migrations
  • mage2vuestorefront
  • vue-storefront/core/store/lib/search.js
  • vue-storefront-api/package.js to modify the restore and migrate scripts
  • docker files
  • add migration scripts to reindex existing indexes to the new format

Later on:

  • What is the expected behavior?

We should have the ES 6.x supported by default

  • What is the motivation / use case for changing the behavior?

The main motivation is that we re bound to 5.x

@pkarw pkarw added this to the 1.5 milestone Sep 8, 2018
@pkarw pkarw mentioned this issue Sep 13, 2018
1 task
@pkarw pkarw modified the milestones: 1.5, 1.6 Oct 23, 2018
@pkarw pkarw modified the milestones: 1.6, 1.7 Nov 7, 2018
@filrak filrak removed this from the 1.7 | Performance milestone Dec 21, 2018
@pkarw pkarw added this to the 2.1 milestone Feb 2, 2019
@pkarw pkarw added feature request Requests for new features. Please be as specific as possible and provide proposal API if it you can P2: Important Priority mark - still high ;) labels Mar 25, 2019
@janmyszkier janmyszkier self-assigned this Apr 25, 2019
@pkarw
Copy link
Collaborator Author

pkarw commented May 1, 2019

@janmyszkier are You working on this one? How does it look like at Your end?

@janmyszkier
Copy link
Contributor

@pkarw not having as much time as I'd like but slowly going forward. So far I managed to make it write to multiple indexes. Will take another shot at it (tomorrow max) and submit a WIP PR so you can all see where we're at

@pkarw
Copy link
Collaborator Author

pkarw commented May 2, 2019

Cool, thanks for the heads up

@janmyszkier
Copy link
Contributor

refactor also needed in

  • vue-storefront/core/lib/search/adapter/api/elasticsearchQuery.js

@janmyszkier
Copy link
Contributor

@pkarw considering the 6.0 is not supported anymore according to https://www.elastic.co/support/eol maybe we should re-do this for 7.1 instead?

@pkarw
Copy link
Collaborator Author

pkarw commented May 28, 2019

Yes, it should more like: most recent stable Elastic version available :)

@pkarw pkarw changed the title Support for Elastic 6.x Support for Elastic 7.x May 28, 2019
@pkarw pkarw modified the milestones: 2.0, 1.11.0-rc.1 Jun 11, 2019
@pkarw pkarw modified the milestones: 1.11.0-rc.1, 1.11.0 Sep 2, 2019
@pkarw
Copy link
Collaborator Author

pkarw commented Sep 3, 2019

@janmyszkier did a great job that can be continued in here:

@janmyszkier
Copy link
Contributor

janmyszkier commented Sep 3, 2019

the current TODO is my opinion is as follows:

Rely on config to decide which format to use
since it is still Proof of Concept none of my repos rely on configs, but should rely on apiVersion value
mage2vuestrorefront: https://github.com/janmyszkier/mage2vuestorefront/blob/master/src/config.js#L57
vue-storefront-api: https://github.com/DivanteLtd/vue-storefront-api/blob/master/config/default.json#L33
vue-storefront: does not have an apiVersion in the config, but IMO, it should. Mainly due to the fact the search in ES requires you to give indexname when searching and while we currently use simply indexName as in https://github.com/DivanteLtd/vue-storefront/compare/master...janmyszkier:jan/es6?expand=1

Update indexer scripts

Migration script
I think we should ditch that since we're going to rely on apiVersion. Simply re-set the store with new release/new config if you want to use different stack. Especially I don't see how you would do no-downtime update in all places at once anyway.

(@pkarw please Note: I also changed initial issue comment as it pointed at vue-storefront/package.json, but it's vue-storefront**-api** where restore and migrate are)

@pkarw
Copy link
Collaborator Author

pkarw commented Sep 13, 2019

https://github.com/DivanteLtd/vue-storefront-api/pull/342/files
vuestorefront/mage2vuestorefront#96

We probably won't gonna make it to Vue Storefront 1.11rc-1 - but we'll try to include the Elastic 7 support in 1.11rc-2 as it's pretty much required for the LTS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requests for new features. Please be as specific as possible and provide proposal API if it you can P2: Important Priority mark - still high ;)
Projects
None yet
Development

No branches or pull requests

3 participants