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

Archive 1.10.0 migrations #2518

Merged

Conversation

arnaldo2792
Copy link
Contributor

Description of changes:
Regardless of the next release version, archiving migrations improves builds' speed. This is similar to what was done in #2357 and #1699.

Testing done:
Builds are successful after archiving the migrations.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@webern
Copy link
Contributor

webern commented Oct 25, 2022

I can think of reasons we might want to keep the last N versions around. Was there a discussion and decision that we will archive migrations right away after a release?

@arnaldo2792
Copy link
Contributor Author

arnaldo2792 commented Oct 25, 2022

Was there a discussion and decision that we will archive migrations right away after a release?

Yes, but wasn't registered anywhere 😿 and it happened too long ago so I don't remember most of the discussion. But we can start a thread now:

I do remember that the argument was that we end up spending time (multiplied by all the variants) building artifacts that already exist in the tuf repos. This also affects the github workflows execution time.

@stmcginnis
Copy link
Contributor

I remember the discussion, but I don't remember where. The key realization for me was that archiving the migrations doesn't mean someone can't upgrade from one of those older versions. The migrations are still available, they just aren't rebuilt every time.

Copy link
Contributor

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

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

lgtm

@etungsten
Copy link
Contributor

I can think of reasons we might want to keep the last N versions around

Can you elaborate on some of them?

@webern
Copy link
Contributor

webern commented Oct 25, 2022

I can think of reasons we might want to keep the last N versions around

Can you elaborate on some of them?

I was mostly thinking about development work in which you want to build a repo with a few earlier versions to check migration paths, but I guess you would build from tags for that anyway, so nevermind.

etungsten
etungsten previously approved these changes Oct 25, 2022
@etungsten etungsten dismissed their stale review October 25, 2022 23:53

The container-runtime settings migration needs to be retroactively fixed

Copy link
Contributor

@webern webern left a comment

Choose a reason for hiding this comment

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

Well, I hadn't considered this eventuality, but it looks like we want to fix a bug in a 1.10.x migration: #2523

I'm marking this as "Request Changes" to pause merging for now.

@arnaldo2792
Copy link
Contributor Author

arnaldo2792 commented Oct 26, 2022

@stmcginnis created #2523 to fix the migration, I'll remove that migration this PR.

@arnaldo2792
Copy link
Contributor Author

(forced push adds back the 1.10.1 migrations, but I forgot to removed them from archive and add them to sources/Cargo.toml)

@arnaldo2792
Copy link
Contributor Author

(forced push adds back v1.10.1 migrations to sources/api/migration/migrations and to sources/Cargo.toml / sources/Cargo.lock)

This helps reduce the build time since the build system currently
compiles everything in the migrations' directory.

Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
@arnaldo2792
Copy link
Contributor Author

(forced push is a rebase onto develop, to make sure I have the fixes in #2523)

@arnaldo2792 arnaldo2792 changed the title Archive 1.10.X migrations Archive 1.10.0 migrations Oct 27, 2022
@arnaldo2792 arnaldo2792 merged commit 0a82497 into bottlerocket-os:develop Oct 27, 2022
@arnaldo2792 arnaldo2792 deleted the archive-1.10-migrations branch June 19, 2023 18:36
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.

4 participants