Disable workflows when scheduler doesn't support it (#5395)#5435
Disable workflows when scheduler doesn't support it (#5395)#5435bstepanovski wants to merge 2 commits into
Conversation
bstepanovski
force-pushed
the
disable-workflows-scheduler-5395
branch
from
92f4830 to
5aa9e88
Compare
bstepanovski
force-pushed
the
disable-workflows-scheduler-5395
branch
from
5aa9e88 to
5424c33
Compare
Bubballoo3
left a comment
Bubballoo3
left a comment
There was a problem hiding this comment.
I think this should include an ENV override flag, in case the check returns a false-negative or a workaround is being developed for an unsupported scheduler. Especially since we don't have a good way of testing this out on different schedulers
|
|
||
| helper_method :workflows_supported? | ||
|
|
||
| def workflows_supported? |
There was a problem hiding this comment.
This method is probably best placed in projects_helper, where you won't need to explicitly define it as a helper method
| clusters = | ||
| if clusters_obj.respond_to?(:values) | ||
| Array(clusters_obj.values) | ||
| elsif clusters_obj.respond_to?(:to_a) | ||
| Array(clusters_obj.to_a) | ||
| elsif clusters_obj.respond_to?(:each) | ||
| clusters_obj.each.to_a | ||
| else | ||
| [] | ||
| end.compact |
There was a problem hiding this comment.
Is this safe accessing of the clusters array duplicated from somewhere else? If each of these options are possible then I imagine we do this elsewhere and it may already be defined elsewhere. Just seems like we are devoting a lot of space to getting the clusters here when that isn't the main focus of the method
github-project-automation
Bot
moved this from Awaiting Review
to Changes Requested
in PR Review Pipeline
bstepanovski
force-pushed
the
disable-workflows-scheduler-5395
branch
from
cad2e31 to
ecab52b
Compare
|
|
||
| # @return [Boolean, nil] configured workflow support override when set, otherwise nil to fall back to auto-detection | ||
| def dashboard_workflows_enabled | ||
| return nil unless ENV.key?('OOD_DASHBOARD_WORKFLOWS_ENABLED') |
There was a problem hiding this comment.
There are only workflows in one place in OOD, so I think the dashboard in the variable name is unnecessary. We can also throw this into boolean methods and we should be able to tell when it is unset (I have to look into that a bit more though)
There was a problem hiding this comment.
Looks like we can't tell whether a bool_config is nil. This seems like it could be a useful feature defining this for all (like Configuration.workflows_enabled_set?) but this is a case where any default value doesn't make much sense, so you can leave it where it's at and just change the variable name
3 participants
Fixes #5395
Turn off workflows when the scheduler doesn’t support them.
Workflow.supported?to check if the current job adapter setup supports workflowsworkflows_supported?helper so controllers and views stay in sync