-
Notifications
You must be signed in to change notification settings - Fork 94
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
actions: fudge /etc/hosts #4176
Conversation
I've just noticed the bash test workflow has no timeout, so it might be worth adding while you're at it diff --git a/.github/workflows/bash.yml b/.github/workflows/bash.yml
index 3ebfc086c..ebd791ed2 100644
--- a/.github/workflows/bash.yml
+++ b/.github/workflows/bash.yml
@@ -10,6 +10,7 @@ on:
jobs:
bash-docker:
runs-on: ubuntu-latest
+ timeout-minutes: 15
strategy:
matrix:
bash-version: |
* ask the host for its names then use those to configure /etc/hosts * nasty hack to deal with broken DNS on GH actions * actions/runner-images#3185
* skip tcp comms if comms method == poll * convert comms method to enum
f6916c2
to
7f895de
Compare
* wait for long enough for the started message to get picked up by the scheduler * this test was passing before because it wasn't testing polling properly (because poll task comms was attempting TCP comms anyway)
* test was not suitible for use with polling task comms
|
||
comms_method = get_comms_method() | ||
return get_runtime_client(comms_method, workflow, timeout=timeout) | ||
return get_runtime_client(get_comms_method(), workflow, timeout=timeout) |
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 seems like a function that doesn't need to exist! Merge with the previous one?
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.
Looks good, tests all passing. It seems actions/runner-images#3185 has been acknowledged as a bug, so we shouldn't need the fudge for long.
🍾 |
@datamel could you make sure you are happy with the comms code changes. (I removed the |
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.
Looks good, I have added a suggestion for logging.
Co-authored-by: Melanie Hall <37735232+datamel@users.noreply.github.com>
2c66506
to
e81a1fe
Compare
Going in with two approvals before any more unrelated bugs show up! |
cylc message
so that it doesn't attempt to send TCP messages whencommunication method = poll
.comms:poll
.Requirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.