-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[console] adding generated spec files for xpack endpoints and adjusting code to… #19928
[console] adding generated spec files for xpack endpoints and adjusting code to… #19928
Conversation
… handle the same override logic
💚 Build Succeeded |
💚 Build Succeeded |
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 tested about 15% of these endpoints and they seemed to work as expected. I found a couple oddities but I don't think they're related to this PR.
"_xpack": { | ||
"url_params": { | ||
"categories": [ | ||
"build", |
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.
"license", | ||
"features" | ||
], | ||
"human": [ |
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.
Good catch @cjcenizal wrong key in override file meant that the override wasn't happening. |
💚 Build Succeeded |
💔 Build Failed |
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.
Just found a few things which may or may not be of note.
@@ -0,0 +1,10 @@ | |||
{ | |||
"xpack.ml.get_categories": { | |||
"data_autocomplete_rules": { |
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.
FWIW, I can only get the autocompleter to suggest these for _xpack/ml/anomaly_detectors/{job_id}/results/categories/{category_id}
, but not _xpack/ml/anomaly_detectors/{job_id}/results/categories
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.
Good catch, generated file had that path end with a slash, fixed it in overrides.
{ | ||
"xpack.ml.get_overall_buckets": { | ||
"data_autocomplete_rules": { | ||
"allow_no_jobs": [true, false], |
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 and exclude_interim
only populate the field with an empty array, instead of [true, false]
. true
and false
are suggested by the autocompleter if you start typing "t" or "f" however.
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.
Should be fixed now, thanks for catching.
{ | ||
"xpack.ml.get_records": { | ||
"data_autocomplete_rules": { | ||
"desc": "__flag__", |
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.
In the xpack.info endpoint, __flag__
is used within a URL parameter, and autocompleter will suggest true
or false
as values for the param. In this case, the autocompleter doesn't suggest these values and instead autofills the value as the string "__flag__"
.
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.
Fixed this here and elsewhere.
"aggregations": {}, | ||
"chunking_config": {}, | ||
"frequency": "", | ||
"indices": [""], |
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 array is not pre-populated with an empty string; it's inserted as an empty array.
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.
fixed
"PUT" | ||
], | ||
"patterns": [ | ||
"_xpack/rollup/job/{id}" |
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.
Any reason "methods" and "patterns" are defined both here and in the generated file?
@cjcenizal thanks for the thorough testing. I have addressed those things. Would you mind taking a look at this one again? |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
…ck_console_autocomplete
💚 Build Succeeded |
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.
Spot tested a handful of endpoints and confirmed that autocomplete kicks in and is correct per spec files.
Code LGTM!
], | ||
"patterns": [ | ||
"_xpack/rollup/job/{id}", | ||
"_xpack/rollup/job/" |
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.
Hitting this endpoint without an id returns an exception, this conflicts with the documentation so is not a console bug, but an Elasticsearch bug (rollup team has been notified and will fix).
…ng code to… (elastic#19928) * adding generated spec files for xpack endpoints and adjusting code to handle the same override logic * accounting for non URL documentation values * fixing issue with incorrect override file * adding support for unknown parameters in path (assuming they are ids) * fixing autocomplete for get_categories ml API * fixing issue with autocomplete for get_overall_buckets * fixing issue with get_overall_buckets autocomplete * fixing get_overall_buckets autocomplete issue for reals * fixing __flag__ in body autocomplete specs * fixing issue with ml put_datafeed autocomplete * removing unnecessary overrides for rollup put_job * fix issue with UrlPatternMatcher * fixing tests * fixing typos * overriding rollup get due to bug in ES specs
…ng code to… (#19928) (#20201) * adding generated spec files for xpack endpoints and adjusting code to handle the same override logic * accounting for non URL documentation values * fixing issue with incorrect override file * adding support for unknown parameters in path (assuming they are ids) * fixing autocomplete for get_categories ml API * fixing issue with autocomplete for get_overall_buckets * fixing issue with get_overall_buckets autocomplete * fixing get_overall_buckets autocomplete issue for reals * fixing __flag__ in body autocomplete specs * fixing issue with ml put_datafeed autocomplete * removing unnecessary overrides for rollup put_job * fix issue with UrlPatternMatcher * fixing tests * fixing typos * overriding rollup get due to bug in ES specs
💚 Build Succeeded |
…ng code to… (#19928) * adding generated spec files for xpack endpoints and adjusting code to handle the same override logic * accounting for non URL documentation values * fixing issue with incorrect override file * adding support for unknown parameters in path (assuming they are ids) * fixing autocomplete for get_categories ml API * fixing issue with autocomplete for get_overall_buckets * fixing issue with get_overall_buckets autocomplete * fixing get_overall_buckets autocomplete issue for reals * fixing __flag__ in body autocomplete specs * fixing issue with ml put_datafeed autocomplete * removing unnecessary overrides for rollup put_job * fix issue with UrlPatternMatcher * fixing tests * fixing typos * overriding rollup get due to bug in ES specs
… handle the same override logic
xpack spec files are now generated in the same fashion as the OSS spec files with overrides to supply body completions where applicable.