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

fix: Pass enable_runner_workflow_job_labels variable to submodule #4195

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Yethal
Copy link

@Yethal Yethal commented Oct 21, 2024

No description provided.

@Yethal Yethal changed the title Pass enable_runner_workflow_job_labels variable to submodule fix: Pass enable_runner_workflow_job_labels variable to submodule Oct 21, 2024
@npalm npalm self-requested a review October 21, 2024 17:59
@stuartp44 stuartp44 marked this pull request as draft October 24, 2024 08:16
@npalm
Copy link
Member

npalm commented Nov 1, 2024

@Yethal just wondering, already had time to work on this PR and get it in ready for review state?

@Yethal Yethal marked this pull request as ready for review November 1, 2024 09:49
@Yethal
Copy link
Author

Yethal commented Nov 1, 2024

@npalm Honestly I don't even remember what was left to do here

@npalm
Copy link
Member

npalm commented Nov 1, 2024

What problem are you trying to fix. I think the parameter is used only by the webhook module. Ci is failing as well.

@Yethal
Copy link
Author

Yethal commented Nov 1, 2024

It doesn't work in multi-runner submodule without this change

@npalm
Copy link
Member

npalm commented Nov 4, 2024

It doesn't work in multi-runner submodule without this change

Please can you have a look on the CI, the change is ginving errors on the mult-runner examples

@npalm
Copy link
Member

npalm commented Nov 15, 2024

@Yethal kind reminder the CI is broken as result of the PR. Would be great if you can have a look. In the mean time I move the PR in draft status. Please mark ready to review when you ar ready.

@npalm npalm marked this pull request as draft November 15, 2024 21:09
@Yethal Yethal marked this pull request as ready for review November 20, 2024 15:36
scale_down_schedule_expression = each.value.runner_config.scale_down_schedule_expression
minimum_running_time_in_minutes = each.value.runner_config.minimum_running_time_in_minutes
runner_boot_time_in_minutes = each.value.runner_config.runner_boot_time_in_minutes
runner_labels = sort(distinct(concat(["self-hosted", each.value.runner_config.runner_os, each.value.runner_config.runner_architecture], each.value.runner_config.runner_extra_labels)))
Copy link
Member

Choose a reason for hiding this comment

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

Please can you rebase and accept changes from main, this change should remain.

@@ -721,3 +721,8 @@ variable "job_retry" {
error_message = "The maxium message delay for SWS is 900 seconds."
}
}

variable "enable_runner_workflow_job_labels_check_all" {
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this to the runner config, so it can be set per runner group.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants