-
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
[Lens] Adds using queries/filters for field existence endpoint #59033
[Lens] Adds using queries/filters for field existence endpoint #59033
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
63b80fd
to
b321b4d
Compare
b933faf
to
f2aca43
Compare
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.
Tested locally, and functionality LGTM. Before merging, please delete one of data.json.gz
or data.json
- you've added the .gz
version without deleting the other.
@@ -113,6 +116,7 @@ async function fetchFieldExistence({ | |||
const docs = await fetchIndexPatternStats({ | |||
fromDate, | |||
toDate, | |||
dslQuery: JSON.parse(dslQuery), |
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.
Nitpick: You've chosen to keep this as a GET request, so the query is passed as a string. Would it make sense to turn this into a POST request with a query body?
@@ -150,7 +151,7 @@ export default ({ getService }: FtrProviderContext) => { | |||
.get( | |||
`/api/lens/existing_fields/${encodeURIComponent( | |||
'logstash-*' | |||
)}?fromDate=${TEST_START_TIME}&toDate=${TEST_END_TIME}` | |||
)}?fromDate=${TEST_START_TIME}&toDate=${TEST_END_TIME}&dslQuery=${DSL_QUERY}` |
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.
Nitpick: dslQuery is now required, but could we provide a default match_all
query? But since this is only used by our client, this is probably fine.
@elasticmachine merge upstream |
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, good to merge!
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* master: (45 commits) skip flaky suite (elastic#59717) UI Metrics use findAll to retrieve all Saved Objects (elastic#59891) [Discover] Migrate Context mocha tests to use Jest (elastic#59658) [Maps] Move redux reducers and store logic to NP (elastic#58294) rebalance x-pack groups (elastic#58930) [Discover] Reimplement $route.reload when index pattern changes (elastic#59877) [Upgrade Assistant Meta] Breaking changes issue template (elastic#59745) Skip CI based on changes in PR (elastic#59939) [ML] Transforms: Replace KqlFilterBar with QueryStringInput. (elastic#59723) [ML] Functional tests - stabilize date_nanos test (elastic#59986) [ML] Typescripting client side endpoint functions (elastic#59928) a11y tests on adding columns to discover table (elastic#59375) fix graph plugin config path (elastic#59540) fix vega config issues (elastic#59737) [Upgrade Assistant] Open And Close Slight Refactor (elastic#59890) [ML] Adding shared services to ml setup contract (elastic#59730) [Visualize] Fix linked search behavior (elastic#59690) [ML] Register NP ML plugin for Kibana management section. (elastic#59762) [Lens] Adds using queries/filters for field existence endpoint (elastic#59033) Delete FilterStateManager and QueryFilter :-D (elastic#59872) ...
Field existence endpoint now uses queries/filters to get the list of fields that contain data.
Fixes #52826
Checklist
Delete any items that are not applicable to this PR.
For maintainers