Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

[WIP] Update elasticsearch connector to handle up to v6 #343

Merged
merged 3 commits into from
Feb 14, 2018

Conversation

n-riesco
Copy link
Contributor

@n-riesco n-riesco commented Jan 17, 2018

TODO

Closes #316
Closes #317

@shannonlal
Copy link
Contributor

@n-riesco I would be interested in working with you on these fixes. Do you want to create a separate branch on the falcon-sql-client that we can merge into? One other todo that I would like to suggest is that we add the ability to define a query for Elastic Search

* Moved elasticsearch specs into their own file.

* Do not catch failures in elasticsearch specs.

* Updated connect spec to test all indices.

* Updated elasticsearchMappings spec to test only the test indices.

* Added scripts to run elasticsearch and datastores specs.
* Added datastores.elasticsearch-v2.spec.js with specs against the
  mocked responses from an Elasticsearch server v2.

* Added script test-unit-elasticsearch-v2.
* Added datastores.elasticsearch-v5.spec.js with specs against the
  mocked responses from an Elasticsearch server v5.

* Added script test-unit-elasticsearch-v5.

* Added Dockerfile to build a container that runs an elasticsearch
  server with some sample indices for testing. This server was used to
  generate the mocked responses.

* The main differences I've found between v2 and v5 is that
  `type: 'string'` in v2 becomes `type: 'text'` in v5.

* The dockerised server uses 5 nodes. The main consequence of this setup
  is that the order of the documents returned by queries may change.

* Also, note that when I created the index/type text-scroll/200k, I had
  to set max_response_window to 200001.

* Added script docker:elasticsearch:build to build a dockerised
  elasticsearch server for testing.

* Added script docker:elasticsearch:start to launched the dockerised
  test server.
@n-riesco
Copy link
Contributor Author

n-riesco commented Feb 9, 2018

@shannonlal I'm sorry I haven't moved this branch into falcon's repo yet. But I didn't want to lose the freedom to do a forced git push, before I got the Dockerfile right.

This is how far I got:

  • I've added tests that use mocked responses from an elasticsearch v2.4 server (to run this tests: yarn test-unit-elasticsearch-v2)

  • I've added tests that use mocked responses from an elasticsearch v5.6.7 server (to run this tests: yarn test-unit-elasticsearch-v5)

  • I've created a docker container that runs an elasticsearch server v5.6.7 with enough sample data to run the tests (to build the image: yarn docker:elasticsearch:build; and to run it: yarn docker:elasticsearch:start).

The main result from the work so far is that Falcon's tests pass with either elasticsearch v2.4 or v5.6.7.

@shannonlal Are you still able to reproduce the errors in this comment?

@shannonlal
Copy link
Contributor

After talking discussion with @n-riesco we agreed to merge this in 💃 .

@n-riesco n-riesco merged commit 8cfc195 into plotly:master Feb 14, 2018
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.

2 participants