Skip to content
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 changes for cancel query not being able to cancel, added back ticks #256

Merged
merged 7 commits into from
Feb 6, 2024
Merged

added changes for cancel query not being able to cancel, added back ticks #256

merged 7 commits into from
Feb 6, 2024

Conversation

sumukhswamy
Copy link
Collaborator

@sumukhswamy sumukhswamy commented Jan 31, 2024

Description

cancel async query for running query
added backtiskc for sample and other queries
#221

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (236db60) 53.04% compared to head (78a029a) 53.04%.
Report is 1 commits behind head on main.

❗ Current head 78a029a differs from pull request most recent head 587e7e4. Consider uploading reports for the commit 587e7e4 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #256   +/-   ##
=======================================
  Coverage   53.04%   53.04%           
=======================================
  Files          33       33           
  Lines        1623     1623           
  Branches      288      288           
=======================================
  Hits          861      861           
  Misses        674      674           
  Partials       88       88           
Flag Coverage Δ
dashboards-query-workbench 53.04% <ø> (ø)

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.

Copy link
Contributor

@RyanL1997 RyanL1997 left a comment

Choose a reason for hiding this comment

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

Hi @sumukhswamy , thanks for putting this together! The change itself LGTM. Could you also fix the Lint? I know it is kinda irrelevant from this change but since the lint rules are targeting on the change files so.

public/components/Main/main.tsx Outdated Show resolved Hide resolved
@@ -533,8 +533,8 @@ export class Main extends React.Component<MainProps, MainState> {
const result: ResponseDetail<string> = this.processQueryResponse(
response as IHttpResponse<ResponseData>
);
const status = result.data.status;
if (_.isEqual(status, 'SUCCESS')) {
const status = result.data.status.toLowerCase();
Copy link
Collaborator

Choose a reason for hiding this comment

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

any specific reason for converting to lower case? Can't we compare directly?

Copy link
Collaborator Author

@sumukhswamy sumukhswamy Feb 1, 2024

Choose a reason for hiding this comment

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

wanted to keep it uniform if there are other changes, initially the status from the back end were in caps

public/components/SQLPage/table_view.tsx Show resolved Hide resolved
Copy link
Collaborator

@paulstn paulstn left a comment

Choose a reason for hiding this comment

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

lgtm

…icks for default and other queries

Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: sumukhswamy <[email protected]>
@sumukhswamy sumukhswamy merged commit 886af5f into opensearch-project:main Feb 6, 2024
9 of 10 checks passed
@opensearch-trigger-bot
Copy link
Contributor

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

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-256-to-2.x.

ps48 pushed a commit to ps48/dashboards-query-workbench that referenced this pull request Feb 6, 2024
…icks (opensearch-project#256)

* added changes for cancel query not being able to cancel, added back ticks for default and other queries

Signed-off-by: sumukhswamy <[email protected]>

* added lint check changes

Signed-off-by: sumukhswamy <[email protected]>

* addressed comments

Signed-off-by: sumukhswamy <[email protected]>

* addressed comments

Signed-off-by: sumukhswamy <[email protected]>

* changed location of types

Signed-off-by: sumukhswamy <[email protected]>

* updated snapshots

Signed-off-by: sumukhswamy <[email protected]>

---------

Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: Sumukh Swamy <[email protected]>
(cherry picked from commit 886af5f)
joshuali925 pushed a commit that referenced this pull request Feb 6, 2024
…icks (#256) (#258)

Signed-off-by: sumukhswamy <[email protected]>
Signed-off-by: Sumukh Swamy <[email protected]>
(cherry picked from commit 886af5f)

Co-authored-by: Sumukh Swamy <[email protected]>
@ps48 ps48 mentioned this pull request Feb 24, 2024
6 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.

6 participants