Skip to content

Start ES in ElasticsearchQueryRunner.main#3322

Closed
findepi wants to merge 1 commit intotrinodb:masterfrom
findepi:findepi/master/start-es-in-elasticsearchqueryrunner-main-0b990a
Closed

Start ES in ElasticsearchQueryRunner.main#3322
findepi wants to merge 1 commit intotrinodb:masterfrom
findepi:findepi/master/start-es-in-elasticsearchqueryrunner-main-0b990a

Conversation

@findepi
Copy link
Member

@findepi findepi commented Apr 3, 2020

Otherwise main() obviously fails.

Otherwise main() obviously fails.
@cla-bot cla-bot bot added the cla-signed label Apr 3, 2020
@martint
Copy link
Member

martint commented Apr 3, 2020

Not if you’re running ES externally... It’s intentional that main() doesn’t start it.

@findepi
Copy link
Member Author

findepi commented Apr 3, 2020

This makes this class usable for everyone, and consistent with main() methods in other runners.

Support for localhost:9200 ES should be an opt-in.
For now, If you already take an effort to run ES manually, you can change the code to use that.
If this is common, we can add switches.

@martint
Copy link
Member

martint commented Apr 3, 2020

This makes this class usable for everyone, and consistent with main() methods in other runners.

Consistency was never a goal. Being able to quickly develop and test changes to the ES connector is.

For now, If you already take an effort to run ES manually, you can change the code to use that.

That's precisely what I wanted to avoid when rewrote the connector -- carrying a set of changes as I'm developing just to be able to connect to a separate instance gets cumbersome and painful quickly.

If this is common

It most definitely is. TPC-H is woefully inadequate to be able to develop and test everything ES and the connector needs to be able to do. You need to load custom data, call ES APIs, etc.

@findepi
Copy link
Member Author

findepi commented Apr 3, 2020

@martint i am closign this

can you tell me how should i use this main method without the change?
maybe we could even add this as a comment there?

@findepi findepi closed this Apr 3, 2020
@findepi findepi deleted the findepi/master/start-es-in-elasticsearchqueryrunner-main-0b990a branch April 3, 2020 16:50
@martint
Copy link
Member

martint commented Apr 3, 2020

maybe we could even add this as a comment there?

I will do that.

I'm also ok with adding a switch -- I just don't want to have to modify code every time I'm testing something.

@martint
Copy link
Member

martint commented Apr 3, 2020

#3331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants