Skip to content

Commit

Permalink
Change priority and destination lists to sets
Browse files Browse the repository at this point in the history
By using sets instead of lists for the valid priority and destination,
the checks for item uniqueness in these datatypes is now handled
inherently, with little detriment.

The only drawback to this is that the system that chooses the default
priority for each tool now simply chooses the first element from
iter(), rather than the one in the middle of the list. This isn't a
huge deal, as that was no more than a guess at what a mid-level
priority would be, and would even cause errors when the chosen
priority wasn't in the tool's default destination section.

Using sets insetad still result in the same issue, so this will need
to be fixed soon, as it prevents certain tool destination configs
from working properly.
  • Loading branch information
Matthew Spelchak committed Feb 5, 2018
1 parent 2628e5f commit c01bff7
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions lib/galaxy/jobs/dynamic_tool_destination.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
list of all valid priorities, inferred from the global
default_desinations section of the config
"""
priority_list = []
priority_list = set()

"""
list of all valid destinations, retrieved from the
job configuration file
"""
valid_destinations = []
valid_destinations = set()


class MalformedYMLException(Exception):
Expand Down Expand Up @@ -785,6 +785,7 @@ def infinite_defaultdict():

if 'default_destination' in obj:
if isinstance(obj['default_destination'], str):
priority_list.add(obj['default_destination'])
if obj['default_destination'] in valid_destinations:
new_config["default_destination"] = obj['default_destination']
else:
Expand All @@ -803,8 +804,7 @@ def infinite_defaultdict():

if isinstance(obj['default_destination']['priority'][priority],
str):
if priority not in priority_list:
priority_list.append(priority)
priority_list.add(priority)

if obj['default_destination']['priority'][priority] in valid_destinations:
new_config['default_destination']['priority'][priority] = obj[
Expand Down Expand Up @@ -1300,11 +1300,12 @@ def map_tool_to_destination(
# For each different rule for the tool that's running
fail_message = None

# set default priority to whatever the middle one is in the list
# The priorities won't necessarily be in order, but it can't be
# worse than choosing a random value (probably)
# assign default priority to the first element that comes out of the
# set. Definitely not the best way to pick fallback default
# destinations for tools, but there's not many other options without
# making default destinations a mandatory field for all tools
if len(priority_list) > 0:
default_priority = priority_list[len(priority_list) / 2]
default_priority = next(iter(priority_list))
priority = default_priority
else:
fail_message = ("No priorities declared in config file!" +
Expand Down Expand Up @@ -1455,7 +1456,7 @@ def map_tool_to_destination(

if destination == "fail":
if fail_message:
raise JobMappingException(fail_message)
raise JobMappingException(fail_message) ###TODO: Should an exception occur when there are no default destinations?
else:
raise JobMappingException(matched_rule["fail_message"])

Expand Down Expand Up @@ -1484,7 +1485,7 @@ def get_valid_destinations_from_config(config_location='/config/job_conf.xml'):
@return: A list of all of the destination IDs declared in the job
configuration file.
"""
valid_destinations = []
valid_destinations = set()

# os.path.realpath gets the path of DynamicToolDestination.py
# and then os.path.join is used to go back four directories
Expand All @@ -1496,7 +1497,7 @@ def get_valid_destinations_from_config(config_location='/config/job_conf.xml'):

for destination in job_conf.getroot().iter("destination"):
if isinstance(destination.get("id"), str):
valid_destinations.append(destination.get("id"))
valid_destinations.add(destination.get("id"))

else:
error = "Destination ID '" + str(destination)
Expand Down

0 comments on commit c01bff7

Please sign in to comment.