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 new gbstats integration if no data returned #1975

Merged
merged 1 commit into from Dec 20, 2023
Merged

Conversation

lukesonnet
Copy link
Contributor

Our recently landed PR #1823 didn't handle the case where no data was returned for 1 or more metrics.

Before that PR, if there was missing data, we just skipped the gbstats call altogether here:

if (!rows || !rows.length) {
return {
unknownVariations: [],
multipleExposures: 0,
dimensions: [],
};
}

After that PR, we removed that check, and were trying to process data with 0 rows in python which was casuing key errors, like this one: https://growthbookapp.slack.com/archives/C02BHUS8NBE/p1702998839752619

Now, we are spinning up python once per analysis, so we need to handle the case where 1 or more metrics are missing data when looping over metrics, which is in gbstats now.

Loom with before and after: https://www.loom.com/share/55099e1de1df400c88de06e77eada25c

@lukesonnet lukesonnet merged commit 22545fd into main Dec 20, 2023
2 of 3 checks passed
@lukesonnet lukesonnet deleted the ls/nodata-gbstats branch December 20, 2023 18:02
Copy link

Your preview environment pr-1975-bttf has been deployed with errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants