-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
feat: Support for elastic.co in ElasticsearchSriver and improved docs #2240
feat: Support for elastic.co in ElasticsearchSriver and improved docs #2240
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2240 +/- ##
==========================================
- Coverage 55.75% 55.68% -0.08%
==========================================
Files 118 118
Lines 8641 8643 +2
Branches 1888 1888
==========================================
- Hits 4818 4813 -5
- Misses 3444 3451 +7
Partials 379 379
Continue to review full report at Codecov.
|
packages/cubejs-elasticsearch-driver/driver/ElasticSearchDriver.js
Outdated
Show resolved
Hide resolved
@hassankhan you probably want to take a look here with updates to docs 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kubarozkwitalski 🙌
Just had some minor nitpicks, if you wouldn't mind addressing them 😄
Co-authored-by: Hassan Khan <contact@hassankhan.me>
Co-authored-by: Hassan Khan <contact@hassankhan.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Hey @ovr , is there anything else I should do to get this merged? |
Thank you @kubarozkwitalski for your contribution 🍰 🥮 |
) * feat: Support for elastic.co instances in Elasticsearch driver and improved docs. * code review fix * Update docs/content/Configuration/Environment-Variables-Reference.md Co-authored-by: Hassan Khan <contact@hassankhan.me> * Update packages/cubejs-elasticsearch-driver/README.md Co-authored-by: Hassan Khan <contact@hassankhan.me> * Update Environment-Variables-Reference.md add link Co-authored-by: Hassan Khan <contact@hassankhan.me>
Check List
Issue Reference this PR resolves
n/a
Description of Changes Made (if issue reference is not provided)
Added support for elastic.co-hosted instances of Elasticsearch. Not tested with opendistro nor native, non-hosted ES.