Multi-job support (using job start/end hooks), optional CloudWatch logging, demo workflows, module/repo rename#2
Closed
ryan-williams wants to merge 0 commit intomainfrom
Closed
Multi-job support (using job start/end hooks), optional CloudWatch logging, demo workflows, module/repo rename#2ryan-williams wants to merge 0 commit intomainfrom
ryan-williams wants to merge 0 commit intomainfrom
Conversation
ryan-williams
force-pushed
the
rw/hooks
branch
2 times, most recently
from
3f0abc9 to
de868e0
Compare
ryan-williams
force-pushed
the
rw/hooks
branch
11 times, most recently
from
4b3f29f to
e09ed8f
Compare
ryan-williams
force-pushed
the
rw/hooks
branch
6 times, most recently
from
6bec165 to
efc6c98
Compare
ryan-williams
force-pushed
the
rw/hooks
branch
4 times, most recently
from
cdb6a54 to
8dcb14b
Compare
ryan-williams
changed the title
ryan-williams
force-pushed
the
rw/hooks
branch
2 times, most recently
from
c648782 to
d4bb7fa
Compare
jder
approved these changes
Aug 11, 2025
Member
jder
left a comment
jder
left a comment
There was a problem hiding this comment.
Awesome stuff, this seems great. I didn't read the bash script super carefully but generally LGTM. Thanks!
Member
There was a problem hiding this comment.
🐑 maybe next time we're in here, it would be nice to swap to a snapshot testing library rather than the many individual asserts on template outputs
src/ec2_gha/__main__.py
Outdated
| check_required(env, required) | ||
| # The timeout is infallible | ||
| timeout = int(os.environ["INPUT_GH_TIMEOUT"]) | ||
| timeout = int(os.environ["INPUT_MAX_INSTANCE_LIFETIME"]) |
Member
There was a problem hiding this comment.
I think this timeout (passed to DeployInstance below) has different semantics from the previous GH_TIMEOUT. Was your thinking that we don't need a timeout for how long to wait for the instance to come up which is separate from the timeout for the instance lifetime?
Member
Author
There was a problem hiding this comment.
Good catch. Added runner_registration_timeout, essentially replacing the old gh_timeout, and used it here.
Comment on lines
+103
to
+110
| # Determine the default user based on the home directory | ||
| if [ "$homedir" = "/home/ubuntu" ]; then | ||
| DEFAULT_USER="ubuntu" | ||
| elif [ "$homedir" = "/home/ec2-user" ]; then | ||
| DEFAULT_USER="ec2-user" | ||
| else | ||
| DEFAULT_USER="root" | ||
| fi |
Member
There was a problem hiding this comment.
Perhaps DEFAULT_USER=$$(stat -c "%U" "$homedir")?
ryan-williams
added a commit
that referenced
this pull request
Aug 16, 2025
Removed workflows not in test branch: - demo-00-minimal.yml - demo-gpu-dbg.yml Added placeholders for workflows from test branch: - demo-gpu-minimal.yml - demo-gpu.yml - demo-multi-instance.yml - demo-multi-job.yml All placeholder workflows display a message that they're being developed in the #2 / rw/hooks branch to avoid confusion. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ryan-williams
added a commit
that referenced
this pull request
Aug 19, 2025
Removed workflows not in test branch: - demo-00-minimal.yml - demo-gpu-dbg.yml Added placeholders for workflows from test branch: - demo-gpu-minimal.yml - demo-gpu.yml - demo-multi-instance.yml - demo-multi-job.yml All placeholder workflows display a message that they're being developed in the #2 / rw/hooks branch to avoid confusion. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ryan-williams
force-pushed
the
rw/hooks
branch
7 times, most recently
from
a90bd76 to
59bd562
Compare
ryan-williams
added a commit
that referenced
this pull request
Aug 20, 2025
Removed workflows not in test branch: - demo-00-minimal.yml - demo-gpu-dbg.yml Added placeholders for workflows from test branch: - demo-gpu-minimal.yml - demo-gpu.yml - demo-multi-instance.yml - demo-multi-job.yml All placeholder workflows display a message that they're being developed in the #2 / rw/hooks branch to avoid confusion. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ryan-williams
force-pushed
the
rw/hooks
branch
5 times, most recently
from
db06efe to
5a95a93
Compare
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ACTIONS_RUNNER_HOOK_JOB_*hooks for job tracking (instead of polling).github/workflows/demo*; seedemos.ymlruns)ec2-gha(and Python module toec2_gha)runner.ymlnow lives in this repo, and wrapsaction.ymlfrom this repo)Examples
demos#18
mamba#15
(these failures are expected, part of demonstrating issues with
pip install mamba_ssmat different versions)Ocean_Emulator#308
CloudWatch console
EC2 instances console (showing many terminat{ed,ing} instances