[autotest] Remove redundant iterable guards (squashed)
The default values are iterable, we dont need this guard.
BUG=None
TEST=None
!Change-Id: I8910e44f45fcdbff5ab03cccef28f3a117756aea
[autotest] Replace or chain with any()
BUG=None
TEST=None
!Change-Id: I647d42164b4c2b872c286dd35f511f9dd7cc6583
[autotest] Inline server_type temp var
BUG=None
TEST=None
!Change-Id: Ic7e57c70c84c8d4c72e741093c8822de05b6f54b
[autotest] Use dict comprehension for atomic_groups_by_name
BUG=None
TEST=None
!Change-Id: I90fc74d3f7d2b4d526cdc60be6d09644a556b439
[autotest] Inline owner temp var
BUG=None
TEST=None
!Change-Id: I4f1d5789b62380334886d2c1ca0d16327ec3f918
[autotest] Extract _get_atomic_group_name_from_labels
BUG=None
TEST=None
!Change-Id: I352c0ae8e355d6dfcca95799a6d1cda2f8f712f2
[autotest] Extract _validate_host_job_sharding
BUG=None
TEST=None
!Change-Id: Iae60e34a2b8e0ff5ae47a33c6b2fead66a5bb749
[autotest] Extract _allowed_hosts_for_master_job
BUG=None
TEST=None
!Change-Id: Ie271ad881e8a5564899408f4898008a134b68176
[autotest] Refactor check in _validate_host_job_sharding
BUG=None
TEST=None
!Change-Id: I5efcc07a2ecaee91ba1d844167f4e43c58b1a8b3
Change-Id: I2da7d4b2e15da4d93b1f8e4b85d365f3bf8fc055
Reviewed-on: https://chromium-review.googlesource.com/424932
Commit-Ready: Allen Li <ayatane@chromium.org>
Tested-by: Allen Li <ayatane@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>
diff --git a/frontend/afe/rpc_utils.py b/frontend/afe/rpc_utils.py
index 6fe70a7..4d808d2 100644
--- a/frontend/afe/rpc_utils.py
+++ b/frontend/afe/rpc_utils.py
@@ -905,58 +905,41 @@
"""
Common code between creating "standard" jobs and creating parameterized jobs
"""
- user = models.User.current_user()
- owner = user.login
-
# input validation
- if not (hosts or meta_hosts or one_time_hosts or atomic_group_name
- or hostless):
- raise model_logic.ValidationError({
- 'arguments' : "You must pass at least one of 'hosts', "
- "'meta_hosts', 'one_time_hosts', "
- "'atomic_group_name', or 'hostless'"
- })
-
+ host_args_passed = any((
+ hosts, meta_hosts, one_time_hosts, atomic_group_name))
if hostless:
- if hosts or meta_hosts or one_time_hosts or atomic_group_name:
+ if host_args_passed:
raise model_logic.ValidationError({
'hostless': 'Hostless jobs cannot include any hosts!'})
- server_type = control_data.CONTROL_TYPE_NAMES.SERVER
- if control_type != server_type:
+ if control_type != control_data.CONTROL_TYPE_NAMES.SERVER:
raise model_logic.ValidationError({
'control_type': 'Hostless jobs cannot use client-side '
'control files'})
+ elif not host_args_passed:
+ raise model_logic.ValidationError({
+ 'arguments' : "For host jobs, you must pass at least one of"
+ " 'hosts', 'meta_hosts', 'one_time_hosts',"
+ " 'atomic_group_name'."
+ })
- atomic_groups_by_name = dict((ag.name, ag)
- for ag in models.AtomicGroup.objects.all())
+ atomic_groups_by_name = {
+ group.name: group for group in models.AtomicGroup.objects.all()
+ }
label_objects = list(models.Label.objects.filter(name__in=meta_hosts))
# Schedule on an atomic group automagically if one of the labels given
# is an atomic group label and no explicit atomic_group_name was supplied.
if not atomic_group_name:
- for label in label_objects:
- if label and label.atomic_group:
- atomic_group_name = label.atomic_group.name
- break
+ atomic_group_name = _get_atomic_group_name_from_labels(label_objects)
+
# convert hostnames & meta hosts to host/label objects
host_objects = models.Host.smart_get_bulk(hosts)
- if not server_utils.is_shard():
- shard_host_map = bucket_hosts_by_shard(host_objects)
- num_shards = len(shard_host_map)
- if (num_shards > 1 or (num_shards == 1 and
- len(shard_host_map.values()[0]) != len(host_objects))):
- # We disallow the following jobs on master:
- # num_shards > 1: this is a job spanning across multiple shards.
- # num_shards == 1 but number of hosts on shard is less
- # than total number of hosts: this is a job that spans across
- # one shard and the master.
- raise ValueError(
- 'The following hosts are on shard(s), please create '
- 'seperate jobs for hosts on each shard: %s ' %
- shard_host_map)
+ _validate_host_job_sharding(host_objects)
+
metahost_objects = []
meta_host_labels_by_name = {label.name: label for label in label_objects}
- for label_name in meta_hosts or []:
+ for label_name in meta_hosts:
if label_name in meta_host_labels_by_name:
metahost_objects.append(meta_host_labels_by_name[label_name])
elif label_name in atomic_groups_by_name:
@@ -991,7 +974,7 @@
else:
atomic_group = None
- for host in one_time_hosts or []:
+ for host in one_time_hosts:
this_host = models.Host.create_one_time_host(host)
host_objects.append(this_host)
@@ -1017,13 +1000,49 @@
test_retry=test_retry,
run_reset=run_reset,
require_ssp=require_ssp)
- return create_new_job(owner=owner,
+
+ return create_new_job(owner=models.User.current_user().login,
options=options,
host_objects=host_objects,
metahost_objects=metahost_objects,
atomic_group=atomic_group)
+def _get_atomic_group_name_from_labels(label_objects):
+ """Get atomic group name from label objects.
+
+ @returns: atomic group name string or None
+ """
+ for label in label_objects:
+ if label and label.atomic_group:
+ return label.atomic_group.name
+
+
+def _validate_host_job_sharding(host_objects):
+ """Check that the hosts obey job sharding rules."""
+ if not (server_utils.is_shard()
+ or _allowed_hosts_for_master_job(host_objects)):
+ shard_host_map = bucket_hosts_by_shard(host_objects)
+ raise ValueError(
+ 'The following hosts are on shard(s), please create '
+ 'seperate jobs for hosts on each shard: %s ' %
+ shard_host_map)
+
+
+def _allowed_hosts_for_master_job(host_objects):
+ """Check that the hosts are allowed for a job on master."""
+ shard_host_map = bucket_hosts_by_shard(host_objects)
+ num_shards = len(shard_host_map)
+ # We disallow the following jobs on master:
+ # num_shards > 1: this is a job spanning across multiple shards.
+ # num_shards == 1 but number of hosts on shard is less
+ # than total number of hosts: this is a job that spans across
+ # one shard and the master.
+ return not (num_shards > 1
+ or (num_shards == 1
+ and len(shard_host_map.values()[0]) != len(host_objects)))
+
+
def encode_ascii(control_file):
"""Force a control file to only contain ascii characters.