-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update gitlab-runner-rs for cancellation support, and timeout configuration, and small cleanups #17
Merged
Conversation
This file contains 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
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
This will pull in cancellation support. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Since gitlab-runner-rs 0.0.7 now supports respecting job cancellation and timeouts from GitLab, so there isn't any reason to add additional timeouts to the API requests anymore. The tests do now need to have some timeouts set on them, since the retries would otherwise go on indefinitely. This slows them down a bit, so the single test function was refactored into 4 more fine-grained, that way they can run in parallel (it's a bit cleaner anyway). Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
At the time I wrote this code I didn't realize that plain `Fn` existed and thought the only two options were `FnOnce` (which I couldn't use since the function is called more than once) and the previously used `FnMut`. This brought the problem that, because the function could mutate state (which we never used outside of the tests), it had to be wrapped in an `Arc<Mutex<_>>`, which was quite ugly. This makes the tests a tad messier since they have to use synchronized operations to keep track of the attempt count now, but it's not too bad and isolated solely to the test code. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
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.
fwiw the first commit is a bit odd as the second one will update the lock file regardless :)
This will allow the user to set the exact timeout to use for the generated monitor jobs, which is useful if they're known to take a long time. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
413a3cb
to
8fcf8c0
Compare
sjoerdsimons
approved these changes
Jan 11, 2024
refi64
added a commit
that referenced
this pull request
Jan 19, 2024
PR #17 added support for job timeouts to the runner. This means that, if your build jobs take a long time to complete, you must now override the timeout via `--job-timeout`. However, because the given option did not exist in previous releases, deploying the updated version becomes more difficult: the new option is not supported by the old version, but the moment you deploy the new version, your builds might time out until the option is passed. To circumvent this, this PR allows pipelines / jobs to set a default timeout in a variable. If the variable is passed to the old runner version, then it will simply ignore it, circumvent the deployment difficulties. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
refi64
added a commit
that referenced
this pull request
Jan 19, 2024
PR #17 added support for job timeouts to the runner. This means that, if your build jobs take a long time to complete, you must now override the timeout via `--job-timeout`. However, because the given option did not exist in previous releases, deploying the updated version becomes more difficult: the new option is not supported by the old version, but the moment you deploy the new version, your builds might time out until the option is passed. To circumvent this, this PR allows pipelines / jobs to set a default timeout in a variable. If the variable is passed to the old runner version, then it will simply ignore it, circumvent the deployment difficulties. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
refi64
added a commit
that referenced
this pull request
Feb 1, 2024
PR #17 added support for job timeouts to the runner. This means that, if your build jobs take a long time to complete, you must now override the timeout via `--job-timeout`. However, because the given option did not exist in previous releases, deploying the updated version becomes more difficult: the new option is not supported by the old version, but the moment you deploy the new version, your builds might time out until the option is passed. To circumvent this, this PR adds support for setting a global timeout for all monitor jobs the runner generates, avoiding the need to urgently make modifications to the pipelines. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
em-
pushed a commit
that referenced
this pull request
Feb 8, 2024
PR #17 added support for job timeouts to the runner. This means that, if your build jobs take a long time to complete, you must now override the timeout via `--job-timeout`. However, because the given option did not exist in previous releases, deploying the updated version becomes more difficult: the new option is not supported by the old version, but the moment you deploy the new version, your builds might time out until the option is passed. To circumvent this, this PR adds support for setting a global timeout for all monitor jobs the runner generates, avoiding the need to urgently make modifications to the pipelines. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 8, 2024
PR #17 added support for job timeouts to the runner. This means that, if your build jobs take a long time to complete, you must now override the timeout via `--job-timeout`. However, because the given option did not exist in previous releases, deploying the updated version becomes more difficult: the new option is not supported by the old version, but the moment you deploy the new version, your builds might time out until the option is passed. To circumvent this, this PR adds support for setting a global timeout for all monitor jobs the runner generates, avoiding the need to urgently make modifications to the pipelines. Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.