feat: Add GitHub timeout #22
Conversation
The purpose of this commit is to add the required changes to enable a configurable timeout on waiting for the runner to become registered with GitHub.
|
This is still work in progress will need to update the README to reflect these changes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #22 +/- ##
==========================================
+ Coverage 94.64% 94.70% +0.05%
==========================================
Files 3 3
Lines 299 302 +3
==========================================
+ Hits 283 286 +3
Misses 16 16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Related to #20 |
This should allow for a bit of a longer wait and should cover most edge cases.
src/gha_runner/gh.py
Outdated
|
|
||
| timeout : int | ||
| The maximum time in seconds to wait for the runner to be online. | ||
| Defaults to 300 seconds. |
src/gha_runner/__main__.py
Outdated
| # Set the default timeout to 10 minutes | ||
| gh_timeout = 1200 |
There was a problem hiding this comment.
comment seems inconsistent with number (assuming this is seconds)
src/gha_runner/__main__.py
Outdated
| # Set the default timeout to 10 minutes | ||
| gh_timeout = 1200 | ||
| # If the user has set the timeout, we will use that instead | ||
| if gh_timeout_var is not None: |
There was a problem hiding this comment.
Checking on the logic here: does this show up as None (not in the environ dict) or does it show up as an empty string (in the environ dict, but not set to a meaningful value)?
If it is actually None, wouldn't the following be an easier way to extract or get default?
gh_timeout = int(os.environ.get("INPUT_GH_TIMEOUT", 1200))
README.md
Outdated
| @@ -38,6 +38,7 @@ For more information see the [self-hosted runner security docs](https://docs.git | |||
| | aws_tags | The AWS tags to use for your runner, formatted as a JSON list. See `README` for more details. | false | | | |||
| | extra_gh_labels | Any extra GitHub labels to tag your runners with. Passed as a comma-separated list with no spaces. | false | | | |||
| | instance_count | The number of instances to create, defaults to 1 | false | 1 | | |||
| | gh_timeout | The amount of timeout for waiting for a runner to be online by GitHub. | false | 1200 | | |||
There was a problem hiding this comment.
maybe some clarification here that this is "seen online by GitHub after cloud providers says it has launched"? Not sure the best way to phrase that, but I'm not sure that the current phrasing would be perfectly clear.
action.yml
Outdated
| @@ -47,6 +47,9 @@ inputs: | |||
| repo: | |||
| description: "The repo to run against. Will use the the current repo if not specified." | |||
| required: false | |||
| gh_timeout: | |||
| description: 'The amount of timeout for waiting for a runner to be online by GitHub.' | |||
There was a problem hiding this comment.
as with the other point on similar text; maybe some more clarification
There was a problem hiding this comment.
Fixed here: abf7339
This is removed because the default _is not_ set by this function but instead set by the application itself.
|
This seems to resolve the changes requested, let me know if there is anything else I can change! |
The purpose of this PR is to add the required changes to enable a configurable timeout on waiting for the runner to become registered with GitHub.