-
Notifications
You must be signed in to change notification settings - Fork 485
Testing different console versions #34345
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
base: main
Are you sure you want to change the base?
Conversation
| # cloud-forward: newest version (index 0) (version in staging) | ||
| # cloud-current: the version released before the latest one (version in prod) | ||
| # cloud-backward: two versions back before the latest released version (version in staging) |
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.
This is not correct I think. We can have the same version in staging and prod at some time.
| minor_versions = get_published_minor_mz_versions(newest_first=True, limit=5) | ||
|
|
||
| # Debug: Print minor versions | ||
| print(f"DEBUG: Minor versions (newest first): {[str(v) for v in minor_versions]}") |
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.
Might want to remove the debug print
| cloud_forward = minor_versions[0] if len(minor_versions) > 0 else None | ||
| cloud_current = minor_versions[1] if len(minor_versions) > 1 else None | ||
| cloud_backward = minor_versions[2] if len(minor_versions) > 2 else None |
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.
You could also assert the length of versions.
|
|
||
| if version_str: | ||
| print(f"Docker image: materialize/materialized:{version_str}") | ||
| c.compose["services"]["materialized"]["image"] = f"materialize/materialized:{version_str}" |
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.
This is not the correct way! It does not respect backwards-incompatible changes. This will fail when testing old versions at some point in the future.
The correct way is:
with c.override(Materialized(image=...)):
and then put the remaining code inside of that context, including the c.up.
Motivation
The console runs sql tests by running the
./mzcompose run defaultscript in CI and for local testing to test the sql queries in the console. Currently we only test against the latest version of Materialize but don't do any testing for previous, current and forward versions. This PR will let us pull the images for different versions and run console tests against these materialize images. Next steps would be to implement a CI job in console repo to test against different materialize versions.Future work : When the console repo is moved into the materialize repo then this script can be expanded to run a test that tests all the versions and pull down the docker images.
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.