-
Notifications
You must be signed in to change notification settings - Fork 924
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
Added framework to get default query string using dataset and language combination #8896
Added framework to get default query string using dataset and language combination #8896
Conversation
Signed-off-by: Amardeepsingh Siglani <[email protected]>
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.
needs tests
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8896 +/- ##
=======================================
Coverage 60.86% 60.86%
=======================================
Files 3802 3802
Lines 91052 91052
Branches 14370 14368 -2
=======================================
+ Hits 55421 55423 +2
+ Misses 32093 32091 -2
Partials 3538 3538
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
src/plugins/data/public/query/query_string/dataset_service/types.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
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 still doesn't make a lot of sense to me with the current existing framework. why would we stop here at the query string level? if the dataset type config being able to override the query syntax provided by the language service then why shouldn't the dataset type config being able to override the editor, fields? it seems like we are overriding the sample queries already so pretty much the language service serves no real purpose if we just keep overriding by the dataset service
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.
are there no unit tests for query_string_manager.ts?
src/plugins/data/public/query/query_string/dataset_service/types.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Amardeepsingh Siglani <[email protected]>
60cb836
to
786547f
Compare
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
Working on tests |
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/public/query/query_string/query_string_manager.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Amardeepsingh Siglani <[email protected]>
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.
approving assuming it passes ci
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-8896-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d3f539c6495f233bf5dc207e727ca05a58c07494
# Push it to GitHub
git push --set-upstream origin backport/backport-8896-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x Then, create a pull request where the |
…e combination (opensearch-project#8896) * added framework to get default query using dataset Signed-off-by: Amardeepsingh Siglani <[email protected]> * Changeset file for PR opensearch-project#8896 created/updated * deduped code Signed-off-by: Amardeepsingh Siglani <[email protected]> * added UTs; minor refactor Signed-off-by: Amardeepsingh Siglani <[email protected]> --------- Signed-off-by: Amardeepsingh Siglani <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
…e combination (opensearch-project#8896) * added framework to get default query using dataset Signed-off-by: Amardeepsingh Siglani <[email protected]> * Changeset file for PR opensearch-project#8896 created/updated * deduped code Signed-off-by: Amardeepsingh Siglani <[email protected]> * added UTs; minor refactor Signed-off-by: Amardeepsingh Siglani <[email protected]> --------- Signed-off-by: Amardeepsingh Siglani <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
This PR adds framework to get the default query for a dataset and language combination from the dataset. If not provided we fallback to the defaults from the language service for each language.
Testing the changes
Tested using local setup
Changelog
Check List
yarn test:jest
yarn test:jest_integration