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

Fix missing result metadata #39201

Merged
merged 14 commits into from Feb 28, 2024
Merged

Fix missing result metadata #39201

merged 14 commits into from Feb 28, 2024

Conversation

kamilmielnik
Copy link
Contributor

@kamilmielnik kamilmielnik commented Feb 27, 2024

Fixes #37009
Fixes #35039

Depends on #39184

Description

Backend won't give us card.result_metadata when query executes for longer than 1.5 s - see PR description for explanation (and Slack discussion). In this case FE needs to explicitly run the query to get this information.

How to verify

Follow reproduction steps from #37009 or #35039 (comment).
Tests are all green.

Stress test for new e2e test passes: https://github.com/metabase/metabase/actions/runs/8079345130

Comment on lines 1063 to 1067
if (question.type() === "model") {
resultsMetadata.columns = ModelIndexes.actions.cleanIndexFlags(
resultsMetadata.columns,
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part should have been moved in #39147.
It worked without moving because it mutates (!) data in redux store.

@kamilmielnik kamilmielnik marked this pull request as ready for review February 27, 2024 13:52
@kamilmielnik kamilmielnik changed the title Fix missing result metadata [WIP] Fix missing result metadata Feb 27, 2024
@kamilmielnik kamilmielnik added the backport Automatically create PR on current release branch on merge label Feb 27, 2024
Copy link

replay-io bot commented Feb 27, 2024

Status Complete ↗︎
Commit 97a2962
Results
⚠️ 5 Flaky
2320 Passed

@kamilmielnik kamilmielnik changed the title [WIP] Fix missing result metadata Fix missing result metadata Feb 27, 2024
@kamilmielnik kamilmielnik requested a review from a team February 27, 2024 15:01
Base automatically changed from 37009-keep-native-question-result-metadata to master February 27, 2024 15:30
@uladzimirdev uladzimirdev requested a review from a team February 28, 2024 07:25
@kamilmielnik kamilmielnik removed the request for review from camsaul February 28, 2024 11:00
@kamilmielnik kamilmielnik enabled auto-merge (squash) February 28, 2024 11:46
@kamilmielnik kamilmielnik merged commit d26b782 into master Feb 28, 2024
110 checks passed
@kamilmielnik kamilmielnik deleted the 37009-fe-result-metadata branch February 28, 2024 11:47
Copy link

@kamilmielnik Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

kamilmielnik added a commit that referenced this pull request Feb 28, 2024
* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Tamás Benkő <tamas@metabase.com>
kamilmielnik added a commit that referenced this pull request Feb 28, 2024
* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Tamás Benkő <tamas@metabase.com>
kamilmielnik added a commit that referenced this pull request Feb 28, 2024
* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>
Co-authored-by: Tamás Benkő <tamas@metabase.com>
kamilmielnik added a commit that referenced this pull request Feb 29, 2024
kamilmielnik added a commit that referenced this pull request Feb 29, 2024
kamilmielnik added a commit that referenced this pull request Mar 1, 2024
* Revert "Fix missing result metadata (#39201)"

This reverts commit d26b782.

* Move resultsMetadata mapping to getSubmittableQuestion

* Bring back the test
kamilmielnik added a commit that referenced this pull request Mar 1, 2024
* Revert "Fix missing result metadata (#39201) (#39243)"

This reverts commit ca5d0e2.

* Bring back e2e test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/QueryingComponents
Projects
None yet
3 participants