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

Require runners to be online when honoring --reuse #626

Merged
merged 10 commits into from
Jul 4, 2021

Conversation

0x2b3bfa0
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 commented Jun 30, 2021

The --reuse option should not consider offline runners as reusable.

@0x2b3bfa0 0x2b3bfa0 added enhancement New feature or request cml-runner Subcommand labels Jun 30, 2021
@0x2b3bfa0 0x2b3bfa0 self-assigned this Jun 30, 2021
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 12:34 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 12:34 Inactive
@0x2b3bfa0 0x2b3bfa0 linked an issue Jun 30, 2021 that may be closed by this pull request
@0x2b3bfa0 0x2b3bfa0 marked this pull request as ready for review June 30, 2021 17:18
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 18:00 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 18:38 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 20:06 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 23:06 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 23:08 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal June 30, 2021 23:18 Inactive
src/drivers/github.js Show resolved Hide resolved
src/drivers/gitlab.js Outdated Show resolved Hide resolved
src/drivers/github.js Outdated Show resolved Hide resolved
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal July 1, 2021 13:36 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal July 1, 2021 14:18 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal July 1, 2021 14:30 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal July 1, 2021 14:46 Inactive
return await Promise.all(
runners.map(async ({ id, name, description, active, online }) => ({
id,
name: name || description,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: name || description,
name: description,

The name property is always null because POST /runners doesn't accept it. We're using description instead of name to store a custom string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say this is true in the meantime GL fixes this bug on their side. The name should be changed and not the description so I would say is ok for now

@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal July 1, 2021 17:20 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal July 1, 2021 19:17 Inactive
It was breaking the tests 🤷🏼‍♂️
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal July 1, 2021 19:26 Inactive
@DavidGOrtega DavidGOrtega merged commit 69d6841 into master Jul 4, 2021
@DavidGOrtega DavidGOrtega deleted the detect-runner-status branch July 4, 2021 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cml-runner Subcommand enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--reuse must check if the runner is offline or online
3 participants