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

[WIP] Async Queries Followup #6966

Merged

Conversation

sejli
Copy link
Member

@sejli sejli commented Jun 7, 2024

Description

  • Stops index pattern from being created when selecting external datasource
  • Checks to see if datasource.name exists before creating dataframe
  • Handles PPL error in request

Issues Resolved

#6957

Screenshot

Testing the changes

Changelog

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

Copy link
Contributor

github-actions bot commented Jun 7, 2024

ℹ️ Manual Changeset Creation Reminder

Please ensure manual commit for changeset file 6966.yml under folder changelogs/fragments to complete this PR.

If you want to use the available OpenSearch Changeset Bot App to avoid manual creation of changeset file you can install it in your forked repository following this link.

For more information about formatting of changeset files, please visit OpenSearch Auto Changeset and Release Notes Tool.

Copy link

codecov bot commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 0% with 28 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature/discover-next@328e08e). Learn more about missing BASE report.

Current head 1b89192 differs from pull request most recent head 471a4ea

Please upload reports for the commit 471a4ea to get more accurate results.

Files Patch % Lines
src/plugins/data/server/search/search_service.ts 0.00% 16 Missing ⚠️
src/plugins/data/public/search/search_service.ts 0.00% 8 Missing ⚠️
...on/search/search_source/fetch/get_search_params.ts 0.00% 2 Missing ⚠️
.../data/common/search/search_source/search_source.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##             feature/discover-next    #6966   +/-   ##
========================================================
  Coverage                         ?   66.94%           
========================================================
  Files                            ?     3452           
  Lines                            ?    68331           
  Branches                         ?    11165           
========================================================
  Hits                             ?    45745           
  Misses                           ?    19966           
  Partials                         ?     2620           
Flag Coverage Δ
Linux_4 34.70% <0.00%> (?)
Windows_1 33.01% <0.00%> (?)
Windows_2 54.88% <0.00%> (?)
Windows_3 44.69% <0.00%> (?)
Windows_4 34.70% <0.00%> (?)

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.

);
// save to cache by title because the id is not unique for temporary index pattern created
indexPatterns.saveToCache(dataSet.title, dataSet);
if (existingIndexPattern) {
Copy link
Member

Choose a reason for hiding this comment

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

actually we want to create a dataset without an existing index pattern or not. the problem is that we try to get indiceswhen we create the temp index pattern even tho s3 is availabe

Copy link
Member

Choose a reason for hiding this comment

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

we want to check the data source if it's like default or not

Comment on lines -248 to -249
if (this.dfCache.get() && this.dfCache.get()?.name !== dataFrame.name) {
scopedIndexPatterns.clearCache(this.dfCache.get()!.name, false);
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't need to clearCache() here right? Think we cleared it in the public folder

@sejli sejli marked this pull request as ready for review June 7, 2024 18:11
@sejli sejli marked this pull request as draft June 7, 2024 19:21
@sejli sejli marked this pull request as ready for review June 7, 2024 22:28
@kavilla kavilla merged commit c86e90c into opensearch-project:feature/discover-next Jun 7, 2024
30 of 53 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/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-6966-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c86e90c91e6e61bd610c52a1c11cc88358588560
# Push it to GitHub
git push --set-upstream origin backport/backport-6966-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 base branch is 2.x and the compare/head branch is backport/backport-6966-to-2.x.

@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/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-6966-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c86e90c91e6e61bd610c52a1c11cc88358588560
# Push it to GitHub
git push --set-upstream origin backport/backport-6966-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 base branch is 2.x and the compare/head branch is backport/backport-6966-to-2.x.

@AMoo-Miki
Copy link
Collaborator

This is not to main and should not be backported.

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.

5 participants