Skip to content

Commit

Permalink
Scottx611x/fix visualization launches (#2775)
Browse files Browse the repository at this point in the history
* Don't raise RuntimeError when fetching url information for all DataSetNodes;
Since all of a DataSets Node information is passed along, its quite likely that there could be a Node with an invalid data file

* Update existing test

* Only check Tool input nodes for valid data file urls

* Add require_valid_url parameter when fetching datafile urls for Tool launch input Nodes;
Visualization Tools also send over information about all other Node's in a DataSet currently, but it shouldn't be the case that a RuntimeError is raised if a url can't be fetched for any of thoses Nodes that aren't explicitly part of a user's selected inputs

* Remove redundant check for valid VisTool input Node's data file urls

* Fix spacing [skip ci]

* Add test coverage
  • Loading branch information
scottx611x committed May 21, 2018
1 parent 40deb27 commit 37800a3
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 24 deletions.
15 changes: 7 additions & 8 deletions refinery/data_set_manager/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1559,15 +1559,14 @@ def test_get_file_url_from_node_uuid_bad_uuid(self):
context.exception.message
)

def test_get_file_url_from_node_uuid_node_with_no_file(self):
def test_get_file_url_from_node_uuid_with_no_file(self):
self.assertIsNone(get_file_url_from_node_uuid(self.node_b.uuid))

def test_get_file_url_from_node_uuid_with_no_file_url_required(self):
with self.assertRaises(RuntimeError) as context:
get_file_url_from_node_uuid(self.node_b.uuid)
self.assertEqual(
"Node with uuid: {} has no associated file url".format(
self.node_b.uuid
),
context.exception.message
)
get_file_url_from_node_uuid(self.node_b.uuid,
require_valid_url=True)
self.assertIn("has no associated file url", context.exception.message)

def test__create_solr_params_from_node_uuids(self):
fake_node_uuids = [str(uuid.uuid4()), str(uuid.uuid4())]
Expand Down
26 changes: 14 additions & 12 deletions refinery/data_set_manager/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1084,16 +1084,19 @@ def update_attribute_order_ranks(old_attribute, new_rank):
return


def get_file_url_from_node_uuid(node_uuid):
def get_file_url_from_node_uuid(node_uuid, require_valid_url=False):
"""
Fetch the full url pointing to a Node's datafile by passing in a Node UUID.
NOTE: Since this method is called within the context of a db transaction,
we are raising exceptions within to nullify said transaction.
:param node_uuid: Node.uuid
:return: a full url pointing to the fetched Node's datafile
:raises: RuntimeError if a Node can't be fetched or if a Fetched Node
has no file associated with it to build a url from
:param require_valid_url: boolean
:return: a full url pointing to the fetched Node's datafile or None
:raises: RuntimeError if a Node can't be fetched or if a valid
datafile url was explicitly required and one wasn't available (Ex: We need
to check a Tool launch's input Nodes in this manner to ensure a Tool
Launch has data file urls to work with)
"""
try:
node = Node.objects.get(uuid=node_uuid)
Expand All @@ -1103,15 +1106,14 @@ def get_file_url_from_node_uuid(node_uuid):
)
else:
url = node.get_relative_file_store_item_url()

if url is None:
raise RuntimeError(
"Node with uuid: {} has no associated file url".format(
node.uuid
if require_valid_url:
if url is None:
raise RuntimeError(
"Node with uuid: {} has no associated file url".format(
node_uuid
)
)
)
else:
return core.utils.get_absolute_url(url)
return core.utils.get_absolute_url(url) if url else None


def fix_last_column(file):
Expand Down
16 changes: 12 additions & 4 deletions refinery/tool_manager/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,10 @@ def update_file_relationships_with_urls(self):
# Append file_uuid to list of FileStoreItem UUIDs
tool_launch_config[self.FILE_UUID_LIST].append(node.file_uuid)

file_url = get_file_url_from_node_uuid(node_uuid)
file_url = get_file_url_from_node_uuid(
node_uuid,
require_valid_url=True
)
tool_launch_config[self.FILE_RELATIONSHIPS_URLS] = (
tool_launch_config[self.FILE_RELATIONSHIPS_URLS].replace(
node_uuid, "'{}'".format(file_url)
Expand Down Expand Up @@ -497,7 +500,8 @@ def get_container_input_dict(self):
self.FILE_RELATIONSHIPS: self.get_file_relationships_urls(),
ToolDefinition.PARAMETERS: self._get_visualization_parameters(),
self.INPUT_NODE_INFORMATION: self._get_detailed_nodes_dict(
self.get_input_node_uuids()
self.get_input_node_uuids(),
require_valid_urls=True # Tool input nodes need valid urls
),
# TODO: adding all of a DataSet's Node info seems excessive. Would
# be great if we had a VisualizationTool using all of this info
Expand Down Expand Up @@ -530,7 +534,8 @@ def _check_max_running_containers(self):
if len(self.django_docker_client.list()) >= max_containers:
raise VisualizationToolError('Max containers')

def _get_detailed_nodes_dict(self, node_uuid_list):
def _get_detailed_nodes_dict(self, node_uuid_list,
require_valid_urls=False):
"""
Create and return a dict with detailed information about all of our
Nodes corresponding to the UUIDs in the given `node_uuid_list`.
Expand All @@ -543,7 +548,10 @@ def _get_detailed_nodes_dict(self, node_uuid_list):
node_info = {
node["uuid"]: {
self.NODE_SOLR_INFO: node,
self.FILE_URL: get_file_url_from_node_uuid(node["uuid"])
self.FILE_URL: get_file_url_from_node_uuid(
node["uuid"],
require_valid_url=require_valid_urls
)
}
for node in solr_response_json["nodes"]
}
Expand Down

0 comments on commit 37800a3

Please sign in to comment.