-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add docker files for easier development setup #7
Conversation
etc/Dockerfile
Outdated
FROM elasticsearch:${ES_VERSION} | ||
# Needs to repeat the ARG here or it won't work | ||
# https://docs.docker.com/engine/reference/builder/#understand-how-arg-and-from-interact | ||
ARG ES_VERSION=8.5.3 |
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.
Wouldn't it be sufficient to just say ARG ES_VERSION
here without repeating the version?
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.
looks like it from the docs I linked but apparently did not read :D Will fix that
etc/es-api-test.http
Outdated
GET http://localhost:9200 | ||
|
||
### show plugins | ||
GET http://localhost:9200/_cat/plugins?v=true&s=component&h=component,version,description |
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.
It would probably be useful to have actually exercise the code path of the plugin here. Perhaps making the same call as in the example of README.md
:
GET http://localhost:9200/_analyze
Content-Type: application/json
{
"tokenizer": "finnish",
"filter": [
{
"type": "raudikko"
}
],
"text": "Testataan raudikon analyysiä tällä tavalla yksinkertaisesti."
}
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.
Isn't that the exact copy of this already?
elasticsearch-analysis-raudikko/README.md
Lines 28 to 33 in 003d1a4
curl -XGET 'localhost:9200/_analyze' -H 'Content-Type: application/json' -d ' | |
{ | |
"tokenizer" : "finnish", | |
"filter" : [{"type": "raudikko"}], | |
"text" : "Testataan raudikon analyysiä tällä tavalla yksinkertaisesti." | |
}' |
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.
Yes it is. I actually generated the request by pasting the original curl request into IDEA.
My point is that if we're going to have a file of executable http-requests, why not include this there? It would feel silly to first run a bunch of requests in one way and then perform additional request in another way.
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.
So I should merge this file with https://github.com/EvidentSolutions/elasticsearch-analysis-raudikko/blob/master/etc/test-analyzer.http ?
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.
There is a slight difference in "filter" [{"type": "raudikko"}]
vs ["raudikko"]
but that might not matter
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.
Oh, right. I completely forgot about the other file. Yeah, it's probably a good idea to have just one file for requests.
Thanks! |
Helps to whip up elasticsearch with freshly built raudikko plugin.
Maybe even better would be to run some integration tests in the build w/ e.g. https://www.testcontainers.org/modules/docker_compose/ but this helped me do manual testing with ES8 support.