Conversation
IvanBM18
changed the title
javanlacerda
left a comment
javanlacerda
left a comment
There was a problem hiding this comment.
Could you provide evidence that it's working in dev? As dev is not sync with master, you will probably need to have a commit that combines your implementation with this fix, and then cherry-pick it over dev.
Added some logs to dev env And here's a screenshot of some logs of tasks in dev that show the feature flag as false: |
letitz
left a comment
letitz
left a comment
There was a problem hiding this comment.
LGTM but you need a test :)
| 'clusterfuzz._internal.base.feature_flags.FeatureFlags' | ||
| ]) | ||
| self.mock._get_task_name.return_value = 'task_name' # pylint: disable=protected-access | ||
| self.mock.FeatureFlags.SWARMING_TASKS.enabled = True |
There was a problem hiding this comment.
We should have a test for create_new_task_request() when the feature flag is false.
| """Returns True if the task is supposed to run on swarming.""" | ||
| # TODO: b/487716733 - Trigger swarming tasks for MAC and Windows | ||
| job = data_types.Job.query(data_types.Job.name == job_name).get() | ||
| if job is None: |
There was a problem hiding this comment.
| if job is None or not feature_flags.FeatureFlags.SWARMING_TASKS.enabled: |
The error we had in prod was when this is_swarming_task is called and the swarming environment is not set. You should check the feature flag here as well to avoid a blast radious error, as the is_swarming_task is being called for every task.
3 participants
Adds a Feature flag so that we are able to control wether tasks in swarming will be able to be triggered or not trough an external value.