Skip to content
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
merged 5 commits into from
Jan 11, 2024

Conversation

refi64
Copy link
Collaborator

@refi64 refi64 commented Jan 10, 2024

No description provided.

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>
@refi64 refi64 requested a review from sjoerdsimons January 10, 2024 22:47
Copy link
Contributor

@sjoerdsimons sjoerdsimons left a 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 :)

src/handler.rs Outdated Show resolved Hide resolved
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>
@refi64 refi64 force-pushed the wip/refi64/cancellation branch from 413a3cb to 8fcf8c0 Compare January 11, 2024 17:42
@sjoerdsimons sjoerdsimons merged commit 73b335b into main Jan 11, 2024
10 checks passed
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>
@em- em- deleted the wip/refi64/cancellation branch February 8, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants