-
Notifications
You must be signed in to change notification settings - Fork 225
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
ci: Retain complete a3p-integration directories and resulting slogfiles #10446
Conversation
03548f6
to
d3782f1
Compare
Deploying agoric-sdk with
|
Latest commit: |
1f88aff
|
Status: | ✅ Deploy successful! |
Preview URL: | https://bacc76cc.agoric-sdk.pages.dev |
Branch Preview URL: | https://gibson-2024-11-retain-a3p-sl.agoric-sdk.pages.dev |
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 won't block merging, but approving isn't something I can do while I'm AFK.
.github/workflows/integration.yml
Outdated
# Copy everything from the $dir to the destination directory | ||
cp -r "$dir"/* "$destination_dir/" | ||
echo "Copied contents of $dir to $destination_dir" | ||
# Copy everything from a3p-integration to /tmp/export/a3p-integration |
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.
Hmmmm. While it is impressive that you are performing this feat with both hands tied behind your back (with Bash under YAML), would it be easier to call a Node.js script leveraging execa
instead?
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.
Meh, no strong feelings either way. I kept it like this as an evolution of what was already there, but am equally happy to change 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.
Okay, let's split the difference, and excise the YAML. Can you factor out the Bash into a file like agoric-sdk/scripts/$something.sh
, and call it from the workflow?
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.
Done.
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.
LGTM! Thanks for extracting export-a3p.sh
... that really tidies up .github/workflows/integration.yml
.
d2ea86e
to
5ff5001
Compare
5ff5001
to
7bbfd9d
Compare
7bbfd9d
to
1f88aff
Compare
…#10466) ## Description Resolving some confusion I experienced while debugging #10165 and working on #10446. * refactor build-submission.sh to be working-directory-aware and accept an arbitrary number of additional arguments * document build-submission.sh w.r.t. package.json "agoricProposal" * document build-all-submissions.sh w.r.t. `./proposals?:*` * add explanatory output to build-all-submissions.sh * explain all of the above in the README ### Security Considerations None known. ### Scaling Considerations n/a ### Documentation Considerations Included in the README and shell scripts. ### Testing Considerations These scripts are covered by integration testing. ### Upgrade Considerations None known; a3p-integration should be self-contained.
Description
Extracted from #10165 and dependent upon Agoric/agoric-3-proposals#193 .
This expands
test-docker-build
CI job artifacts from just core eval scripts to their containing a3p-integration directory plus the job's slogfile.Security Considerations
n/a
Scaling Considerations
The final slogfile from #10165 was about 67 MB, which is smaller than the 161 MB deployment-test-results artifact. But regardless, I've limited retention to
4 days on success/10 days on failure10 days, and we could also compress before upload if even that is too much.Documentation Considerations
n/a
Testing Considerations
n/a
Upgrade Considerations
n/a