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(cache): Fix two bugs in Admin / Performance #44255

Merged
merged 7 commits into from
Jun 19, 2024
Merged

Conversation

rafpaf
Copy link
Contributor

@rafpaf rafpaf commented Jun 14, 2024

Closes #44249: When a new minimum query duration is added to an adaptive policy, it disappears from the form on save.
Closes #44257: Aberrant edge case: 12:00PM cannot be used as a time in the Schedule component.

Copy link
Contributor Author

rafpaf commented Jun 14, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rafpaf and the rest of your teammates on Graphite Graphite

@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label Jun 14, 2024
@rafpaf rafpaf changed the title fix(cache): Add e2e tests to Admin / Performance and fix two bugs: fix(cache): Add e2e tests to Admin / Performance and fix two bugs Jun 14, 2024
@rafpaf rafpaf added the backport Automatically create PR on current release branch on merge label Jun 14, 2024 — with Graphite App
@rafpaf rafpaf changed the title fix(cache): Add e2e tests to Admin / Performance and fix two bugs [WIP] fix(cache): Add e2e tests to Admin / Performance and fix two bugs Jun 14, 2024
@rafpaf rafpaf marked this pull request as ready for review June 14, 2024 21:53
Copy link

graphite-app bot commented Jun 14, 2024

Graphite Automations

"Warn authors when publishing large PRs" took an action on this PR • (06/14/24)

1 teammate was notified to this PR based on Raphael Krut-Landau's automation.

Copy link

replay-io bot commented Jun 14, 2024

Status Complete ↗︎
Commit 981d761
Results
⚠️ 4 Flaky
2649 Passed

@rafpaf rafpaf changed the title [WIP] fix(cache): Add e2e tests to Admin / Performance and fix two bugs fix(cache): Add e2e tests to Admin / Performance and fix two bugs Jun 15, 2024
@rafpaf rafpaf requested a review from a team June 15, 2024 02:26
rafpaf and others added 6 commits June 19, 2024 11:54
- When adaptive policy is saved, it's not shown on the page
- 12:00PM cannot be used as a time in the Schedule component
- Clear cache when switching strategy to "Don't cache results"
@rafpaf rafpaf changed the title fix(cache): Add e2e tests to Admin / Performance and fix two bugs fix(cache): Fix two bugs in Admin / Performance Jun 19, 2024
@npfitz
Copy link
Contributor

npfitz commented Jun 19, 2024

#44249 Still seems to be broken

Copy link
Contributor

@npfitz npfitz left a comment

Choose a reason for hiding this comment

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

Checked that the issues mentioned were resolved, and they seem to be. I don't entirely follow all the conversion logic though. Will catch up with @rafpaf when he's back to get an overview

@rafpaf rafpaf dismissed sloansparger’s stale review June 19, 2024 22:51

Nick has now approved this PR after Sloan's changes

@rafpaf rafpaf merged commit 08613cb into master Jun 19, 2024
111 checks passed
@rafpaf rafpaf deleted the fix-adaptive-policy-bug branch June 19, 2024 22:53
rafpaf added a commit that referenced this pull request Jun 19, 2024
Ensures that minimum query durations are correctly displayed in the form post-save (#44249) and that all times can be used in the Schedule component  (#44257)
rafpaf added a commit that referenced this pull request Jun 20, 2024
* fix(cache): Fix two bugs in Admin / Performance (#44255)

Ensures that minimum query durations are correctly displayed in the form post-save (#44249) and that all times can be used in the Schedule component  (#44257)

---------

Co-authored-by: Raphael Krut-Landau <[email protected]>
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/AdminWebapp Admin and Webapp team
Projects
None yet
3 participants