Skip to content

[Auto Suggest] DQL autosuggest with ANTLR#7391

Merged
kavilla merged 47 commits intoopensearch-project:mainfrom
paulstn:antlr-autocomplete
Jul 30, 2024
Merged

[Auto Suggest] DQL autosuggest with ANTLR#7391
kavilla merged 47 commits intoopensearch-project:mainfrom
paulstn:antlr-autocomplete

Conversation

@paulstn
Copy link
Collaborator

@paulstn paulstn commented Jul 23, 2024

Description

Implements ANTLR based autocomplete for DQL queries, currently within Discover.

Ignore files located in .generated and grammar/.antlr

Previously reviewed/approved in #7467 to be merged into the feature/discover-2.0-1 branch. Was then reverted to isolate test failures, and then moved to this pr to get merged into main directly.

Screenshot

Testing the changes

Changelog

  • feat: DQL Autocomplete

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

opensearch-changeset-bot bot added a commit to paulstn/OpenSearch-Dashboards that referenced this pull request Jul 23, 2024
@codecov
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

Attention: Patch coverage is 58.68794% with 233 lines in your changes missing coverage. Please review.

Project coverage is 63.64%. Comparing base (7a9e9ed) to head (d3db8da).
Report is 229 commits behind head on main.

Files with missing lines Patch % Lines
...gins/data/public/antlr/dql/.generated/DQLParser.ts 55.76% 174 Missing and 18 partials ⚠️
...c/plugins/data/public/antlr/dql/code_completion.ts 70.12% 9 Missing and 14 partials ⚠️
...ugins/data/public/antlr/dql/.generated/DQLLexer.ts 76.47% 8 Missing ⚠️
...ashboards_react/public/code_editor/code_editor.tsx 36.36% 3 Missing and 4 partials ⚠️
...s/data/public/autocomplete/autocomplete_service.ts 0.00% 2 Missing ⚠️
src/plugins/data/public/plugin.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7391      +/-   ##
==========================================
- Coverage   63.67%   63.64%   -0.04%     
==========================================
  Files        3630     3634       +4     
  Lines       79545    80099     +554     
  Branches    12609    12682      +73     
==========================================
+ Hits        50652    50977     +325     
- Misses      25819    26015     +196     
- Partials     3074     3107      +33     
Flag Coverage Δ
Linux_1 30.56% <11.31%> (-0.19%) ⬇️
Linux_2 55.58% <ø> (ø)
Linux_3 40.58% <59.13%> (+0.20%) ⬆️
Linux_4 31.32% <11.80%> (-0.24%) ⬇️
Windows_1 30.58% <11.31%> (-0.19%) ⬇️
Windows_2 55.53% <ø> (ø)
Windows_3 40.58% <59.13%> (+0.21%) ⬆️
Windows_4 31.32% <11.80%> (-0.24%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sejli
sejli previously approved these changes Jul 23, 2024
Copy link
Member

@sejli sejli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, have some nits and one thing regarding http.

paulstn pushed a commit to paulstn/OpenSearch-Dashboards that referenced this pull request Jul 23, 2024
@paulstn paulstn force-pushed the antlr-autocomplete branch from 85ee824 to d3bd0ce Compare July 23, 2024 21:36
paulstn added 16 commits July 30, 2024 11:23
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
…and utilize selectionEnd if no position

Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
* update code completion to not return for visualize

Signed-off-by: Paul Sebastian <paulstn@amazon.com>

* update types to match completionitemkind

Signed-off-by: Paul Sebastian <paulstn@amazon.com>

---------

Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq in the PR

this.props.editorWillMount();
}

monaco.languages.onLanguage(this.props.languageId, () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have we verify no performance issues? that means anytime this is re-rendered its going to re register

* The language ID, meant to restrict the specified configuration for only this language. When
* not provided, will apply the language configuration for every language.
*/
language?: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

little bit confused, but you have more context than me. it seems like the only time this is set is in the loop where we check the language. and if we have to register the languages so we would still have to pass this prop. so as long as im registering the language then i can register the language configuration?

}
};

private fetchIndexPatterns = debounce(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i believe index patterns service might be easier. it's ok can be fast follow

ashwin-pc
ashwin-pc previously approved these changes Jul 30, 2024
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
kavilla
kavilla previously approved these changes Jul 30, 2024
ashwin-pc
ashwin-pc previously approved these changes Jul 30, 2024
paulstn added 2 commits July 30, 2024 15:15
Signed-off-by: Paul Sebastian <paulstn@amazon.com>
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.16 failed:

The process '/usr/bin/git' failed with exit code 128

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.16 2.16
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.16
# Create a new branch
git switch --create backport/backport-7391-to-2.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2b1d01f4aa87a91b0fe1ec2cfd0ee9653adb02c5
# Push it to GitHub
git push --set-upstream origin backport/backport-7391-to-2.16
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.16

Then, create a pull request where the base branch is 2.16 and the compare/head branch is backport/backport-7391-to-2.16.

@paulstn paulstn mentioned this pull request Jul 31, 2024
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants