-
-
Notifications
You must be signed in to change notification settings - Fork 258
Make 'pelias elastic stats' use index from pelias.json instead of static index name #372
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
Make 'pelias elastic stats' use index from pelias.json instead of static index name #372
Conversation
cmd/elastic.sh
Outdated
| function elastic_stats(){ | ||
| curl -s "http://${ELASTIC_HOST:-localhost:9200}/pelias/_search?request_cache=true&timeout=10s&pretty=true" \ | ||
| #Extract the API section from the pelias.json | ||
| api_section=$(cat "$DATA_DIR/../pelias.json" | sed -n '/"api": {/,/}/p') |
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.
The pelias.json file is not always relative to the $DATA_DIR.
cmd/elastic.sh
Outdated
| #Extract the API section from the pelias.json | ||
| api_section=$(cat "$DATA_DIR/../pelias.json" | sed -n '/"api": {/,/}/p') | ||
| #Extract the value of api.indexName | ||
| index_name=$(echo "$api_section" | grep '"indexName"' | sed 's/.*"indexName": "\(.*\)".*/\1/') |
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.
I'd prefer to avoid using sed if possible, jq would be good option if it didn't introduce an external dependency. maybe node -e is the best option for this and the above?
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.
This works, but only when the node_modules directory is present, which it isn't for the docker bash scripts 😿
node -e "console.log(require('pelias-config').generate().schema.indexName)"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.
Maybe it's best to move this into the pelias/schema module and then call it through docker?
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, should resolve all the current problems.
|
Blocked by pelias/schema#507 |
|
Sorry to flip-flop so much on this, would this small change suffice? diff --git a/cmd/elastic.sh b/cmd/elastic.sh
index 701c19c..030b163 100644
--- a/cmd/elastic.sh
+++ b/cmd/elastic.sh
@@ -62,7 +62,7 @@ function elastic_info(){ curl -s "http://${ELASTIC_HOST:-localhost:9200}/"; }
register 'elastic' 'info' 'display elasticsearch version and build info' elastic_info
function elastic_stats(){
- curl -s "http://${ELASTIC_HOST:-localhost:9200}/pelias/_search?request_cache=true&timeout=10s&pretty=true" \
+ curl -s "http://${ELASTIC_HOST:-localhost:9200}/${ELASTIC_INDEX:-'pelias'}/_search?request_cache=true&timeout=10s&pretty=true" \
-H 'Content-Type: application/json' \
-d '{
"aggs": { |
|
The last commit loops the parameter, if present, in the schema container, so it is possible to use the same syntax. If no indexName is given in the config, it falls back to defaults.json, which is pelias. Personally, I prefer this solution because it eliminates a static name, which was the reason for this PR in the first place. |
👋 I did some awesome work for the Pelias project and would love for everyone to have a look at it and provide feedback.
Here's the reason for this change 🚀
pelias elastic statshas the default index hardcoded #371Here's what actually got changed 👏
The code is a little bit hacky but I thought it was reasonable to not relay on external tools like jq.
Here's how others can test the changes 👀
pelias.jsonlikepelias elastic statsShould now show
Results for index "everything except pelias":and the results for the index "everything except pelias".