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

Update DB schema for workflows non-root migration #6351

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Conversation

bduffany
Copy link
Member

// DefaultNonRootRunner decides whether workflow and remote bazel runs
// within this repository should run as a non-root user by default.
// TODO(http://go/b/3286): Remove this field after completing migration.
DefaultNonRootRunner bool `gorm:"not null;default:false"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change to default:0

Bools are stored as tiny ints. There's a bug in the gorm library where it thinks a migration is occurring where the default value is changing from false -> 0

Copy link
Member Author

@bduffany bduffany Apr 11, 2024

Choose a reason for hiding this comment

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

I asked @tempoz for help understanding this - my understanding of the situation is

  • We used to store bools as tinyint in mysql (6b2e537)
  • postgres does not support tinyint, so we had to change those to bool
  • For those existing fields we need to keep them as 0/1 which is compatible with both bool and tinyint, so mysql does not try to migrate from tinyint to bool
  • For new bool fields, it's fine to use false/true

I'm gonna make this numeric anyway just for consistency though. At some point it might be worth either migrating the MySQL columns to be proper bools (if it's inexpensive) or adding a test that ensures we consistently use 0/1 for bool defaults

@bduffany bduffany merged commit 6584e0f into master Apr 12, 2024
18 checks passed
@bduffany bduffany deleted the wf-nonroot-default branch April 12, 2024 16:32
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

2 participants