-
Notifications
You must be signed in to change notification settings - Fork 432
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
Setup Experience Migration #22405
Setup Experience Migration #22405
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat-setup-experience #22405 +/- ##
=========================================================
+ Coverage 65.21% 65.22% +0.01%
=========================================================
Files 1496 1498 +2
Lines 117157 117228 +71
Branches 3480 3480
=========================================================
+ Hits 76399 76465 +66
+ Misses 33640 33638 -2
- Partials 7118 7125 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
return fmt.Errorf("failed to add install_during_setup to vpp_apps_teams: %w", err) | ||
} | ||
|
||
_, err = tx.Exec(`ALTER TABLE host_script_results ADD install_during_setup BOOL NOT NULL DEFAULT false`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth adding a foreign key column to host_script_results pointing to the new setup experience scripts table in the other file instead of a bool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upside is that it's clear which setup script it belongs to without searching the other table for the row that references this row. The downside is that once the script is deleted and we set the foreign key in the host_script_results
table to NULL
, we no longer know that the script was run by a setup script, and it now just looks like an ad-hoc script run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roperzh Going with foreign key for now, would appreciate any opinions on this
#22375
changes/
,orbit/changes/
oree/fleetd-chrome/changes
.See Changes files for more information.
COLLATE utf8mb4_unicode_ci
).