-
Notifications
You must be signed in to change notification settings - Fork 6
Allow disabling ISOs in deployment script #308
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
Conversation
With a list of items to include for the distributed version, we no longer have a good reason to collect all items into a tmp folder and upload it recursively. Doing so uses - more lines of code (cp to folder, then cp to S3), - more time (copy large files on disk), - more disk space (duplicated files). While the awscli S3 client parallelizes uploads, this already happens when invoked for a single file of substantial size (concurrent upload of multipart chunks)[^1] and this should max out the connection. So we seem to have only downsides and no benefits from collecting into a local directory first. [^1]: https://docs.aws.amazon.com/cli/latest/topic/s3-config.html#multipart-chunksize
This automates two simple checks that act as sanity checks that the deployed version truly went "live". They were previously part of a manual release checklist, but can be easily checked by a computer. Because they run after the deployment there seems to be no sense in failing the script, but the verification outcome is color coded to stand out.
Drive-by-comment: normalize this with a |
Could consider, yes. Also thought of a verbose mode, in which it includes all items in the summary. |
Added this, it only skips uploads during dry-run and actually performs any local operations like signing, so one can check that it works. Also added a verbose option. |
|
Building with options akin to Checks fail, because
|
dividat-jgu
left a comment
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.
Good change! Testing it out now, having trouble building develop for the moment.
| else: | ||
| print(f"\033[91mFAIL\033[0m [{description}] Expected {expected}, got {actual}") | ||
| except Exception as e: | ||
| print(f"\033[91mFAIL\033[0m [{description}] Exception: {e}") |
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.
Nitpick: create small helpers for color codes? Like colorizeSuccess(txt: str) -> str and colorizeError(txt: str) -> 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.
I decided not to, because there's not lots of duplication right now, but I also don't mind adding it.
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’s a bit hard to see at a glance the message, or if there is an error with the codes.
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.
A bit on the edge, but maybe it's worth it:
To avoid manually optimizing for existing or absent slashes in path fragments, use a helper that strips and adds slashes appropriately. Similar functions exist in Python stdlibs, but are not quite right: - https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urljoin performs relative path resolution instead of just joining - https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.joinpath is sensitive to leading slashes (absolute paths) in parts
|
✔️ Skipping isos doesn’t create links in |
It's not a status, and it doesn't follow the same format. Retreating to a simple print before ending up with an AbstractPrintStatusProxyBeanFactory.
Lay out development plan section closer to other sections.
dividat-jgu
left a comment
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.
Looking good, deploy to develop after the PR has been merged?
Triggered by wanting to make
deploy-updaterespectbuildLiveandbuildInstaller(avoiding to build and publish ISOs), along the wayTesting
Handy diff to inspect what script would do:
Checklist