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 migrations and add migration CI checks #75

Merged
merged 29 commits into from
Nov 22, 2023

Conversation

liamaharon
Copy link
Contributor

@liamaharon liamaharon commented Oct 24, 2023

Closes #70

The error with Asset Hub Kusama will be fixed when this commit lands in the runtime.

@liamaharon liamaharon changed the title [WIP] Runtime migration CI [WIP] Check runtime migrations CI Oct 24, 2023
@liamaharon liamaharon changed the title [WIP] Check runtime migrations CI Check runtime migrations CI Oct 24, 2023
Copy link
Contributor

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Ty! Looking good, just some questions.

.github/workflows/check-migrations.yml Show resolved Hide resolved
fi
./try-runtime \
--runtime ./${{ matrix.runtime.path }}/target/srtool/production/wbuild/$PACKAGE_NAME/$RUNTIME_BLOB_NAME \
on-runtime-upgrade --checks=pre-and-post $NO_WEIGHT_WARNINGS_FLAG live --uri ${{ matrix.runtime.uri }}
Copy link
Contributor

Choose a reason for hiding this comment

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

We not all checks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The disabled checks here are try-state invariants. I still need to test those on all chains to make sure they pass. Don't want to turn it on yet just in case there're some jobs fail when in reality everything about the migrations is fine.

Copy link
Member

Choose a reason for hiding this comment

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

You could also first create a snap and then once run with all checks and once with only pre-and-post. The first one would only be good for logging, not erroring.

.github/workflows/check-migrations.yml Outdated Show resolved Hide resolved
@liamaharon
Copy link
Contributor Author

fyi the error with Asset Hub Kusama will be fixed when this commit lands in the runtime.

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Nice1

.github/workflows/check-migrations.yml Show resolved Hide resolved
.github/workflows/check-migrations.yml Outdated Show resolved Hide resolved
.github/workflows/check-migrations.yml Show resolved Hide resolved
fi
./try-runtime \
--runtime ./${{ matrix.runtime.path }}/target/srtool/production/wbuild/$PACKAGE_NAME/$RUNTIME_BLOB_NAME \
on-runtime-upgrade --checks=pre-and-post $NO_WEIGHT_WARNINGS_FLAG live --uri ${{ matrix.runtime.uri }}
Copy link
Member

Choose a reason for hiding this comment

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

You could also first create a snap and then once run with all checks and once with only pre-and-post. The first one would only be good for logging, not erroring.

@liamaharon liamaharon mentioned this pull request Nov 21, 2023
5 tasks
@liamaharon
Copy link
Contributor Author

Thank you for the reviews :)

I will fix the failing migrations, then this will be ready to merge.

@liamaharon liamaharon changed the title Check runtime migrations CI Fix migrations and add migration CI Nov 22, 2023
@liamaharon liamaharon changed the title Fix migrations and add migration CI Fix migrations and add migration CI checks Nov 22, 2023
system-parachains/asset-hubs/asset-hub-kusama/src/lib.rs Outdated Show resolved Hide resolved
system-parachains/bridge-hubs/bridge-hub-kusama/src/lib.rs Outdated Show resolved Hide resolved
relay/polkadot/src/lib.rs Outdated Show resolved Hide resolved
relay/polkadot/src/lib.rs Outdated Show resolved Hide resolved
relay/kusama/src/lib.rs Outdated Show resolved Hide resolved
@bkchr bkchr merged commit 569787c into polkadot-fellows:main Nov 22, 2023
12 of 13 checks passed
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.

Add try-runtime CI job for checking the migrations
6 participants