config: Enforce the 22-character-limit imposed by Taskcluster #19

Merged
merged 1 commit into from Oct 24, 2016

Projects

None yet

3 participants

@JohanLorenzo
Contributor

Like discussed in this comment, taskcluster doesn't allow more than 22 characters for IDs. If an instance is configured with an invalid ID, the failure occurs only when the first task is being handled. As @catlee suggested offline, we can fail earlier, like when the configuration is parsed.

@coveralls

Coverage Status

Coverage remained the same at 100.0% when pulling 335f824 on JohanLorenzo:enforce-22-characters into b7a5a99 on mozilla-releng:master.

@JohanLorenzo
Contributor
@escapewindow

Awesome, thank you!

scriptworker/test/test_config.py
@@ -74,6 +74,19 @@ def test_check_config_bad_keyring(t_config):
assert "needs to start with %(gpg_home)s/" in "\n".join(messages)
+def test_check_config_invalid_ids(t_config):
@escapewindow
escapewindow Oct 24, 2016 Member

You could have used pytest.mark.parametrize to specify the key (e.g. provisioner_id) and the expected message (e.g., provisioner_id doesn\'t match "^[a-zA-Z0-9-_]{1,22}$" (required by Taskcluster)). I don't think it's a big deal.

I do love that you added tests!

scriptworker/test/test_config.py
+ t_config['worker_id'] = 'twenty-three-characters'
+
+ messages = config.check_config(t_config, "test_path")
+ assert 'provisioner_id doesn\'t match "^[a-zA-Z0-9-_]{1,22}$" (required by Taskcluster)' in "\n".join(messages)
@escapewindow
escapewindow Oct 24, 2016 Member

I don't think you need to join the messages with '\n'; member in list works as long as member is a complete message. Also, you join messages with '\n' multiple times. I also don't think this is a huge issue.

@JohanLorenzo JohanLorenzo config: Enforce the 22-character-limit imposed by Taskcluster
80b14fd
@coveralls

Coverage Status

Coverage remained the same at 100.0% when pulling 80b14fd on JohanLorenzo:enforce-22-characters into b7a5a99 on mozilla-releng:master.

@escapewindow escapewindow merged commit e4ced86 into mozilla-releng:master Oct 24, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment