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 issue with forms after cleanup #15332

Merged
merged 2 commits into from
Jan 9, 2025
Merged

Fix issue with forms after cleanup #15332

merged 2 commits into from
Jan 9, 2025

Conversation

adrinr
Copy link
Collaborator

@adrinr adrinr commented Jan 9, 2025

Description

Fixing an issue introduced in #15295
We did change the way that some datafetches are consumed, assuming we always have a valid datasource within it. I skipped a usage of it, where this was not the case.

Error we got:
image

After the fix:
image

Copy link

qa-wolf bot commented Jan 9, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/xs labels Jan 9, 2025
@adrinr adrinr requested a review from aptkingston January 9, 2025 13:36
@adrinr adrinr merged commit e4689e9 into master Jan 9, 2025
20 checks passed
@adrinr adrinr deleted the fix-form-datasources branch January 9, 2025 14:05
@github-actions github-actions bot locked and limited conversation to collaborators Jan 9, 2025
@@ -31,7 +31,7 @@ const getDatasourceFetchInstance = datasource => {
if (!handler) {
return null
}
return new handler({ API })
return new handler({ API, datasource })
Copy link
Member

Choose a reason for hiding this comment

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

This has the effect of causing the fetch to load initial data, which means any usage of fetchDatasourceSchema or fetchDatasourceDefinition (which are quite common) will cause a wasted call to the search endpoint. It's quite spammy and should definitely be fixed before releasing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firestorm Data/Infra/Revenue Team size/xs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants