Scottx611x/persistent visualization urls #2638
Conversation
refinery/tool_manager/tests.py
Outdated
| assign_perm('core.read_dataset', self.user, self.tool.dataset) | ||
|
|
||
| # Need to set_password() to be able to login. Otherwise | ||
| # usr.password in the hash representation which is not what the |
There was a problem hiding this comment.
"in the hash" -> "is the hash"?
|
|
||
| with mock.patch.object(VisualizationTool, "launch") as launch_mock: | ||
| get_response = self.client.get( | ||
| "{}/".format(self.tool.get_relative_container_url()) |
There was a problem hiding this comment.
Is the trailing slash required?
There was a problem hiding this comment.
It is unfortunately, Normal django routing middleware tacks this on automatically. The format() is probably overkill for this though
refinery/tool_manager/views.py
Outdated
| @@ -164,3 +168,59 @@ def relaunch(self, request, *args, **kwargs): | |||
| except Exception as e: | |||
| logger.error(e) | |||
| return HttpResponseBadRequest(e) | |||
|
|
|||
|
|
|||
| class VisualizationToolProxy(Proxy, object): | |||
There was a problem hiding this comment.
It was already a visualization-tool-proxy: Perhaps rename to reflect the difference? AutoRelaunchProxy or something like that.
There was a problem hiding this comment.
Would this be better left in the file where it's used? Or would that be weird?
There was a problem hiding this comment.
Agree with the renaming, I'd push to keep it in views.py since we're wrapping a view now
refinery/tool_manager/utils.py
Outdated
|
|
||
| def user_has_access_to_tool(user, tool): | ||
| return \ | ||
| False if not user.has_perm('core.read_dataset', tool.dataset) else True |
There was a problem hiding this comment.
just return user.has_perm('core.read_dataset', tool.dataset)?
refinery/tool_manager/views.py
Outdated
| return HttpResponseForbidden( | ||
| "Requesting User does not have sufficient permissions to " | ||
| "view Tool with uuid: {}".format(visualization_tool.uuid) | ||
| ) |
There was a problem hiding this comment.
Could be more concise? "User does not have permission to view Tool {}"
|
(But test failures could be related?) |
Codecov Report
@@ Coverage Diff @@
## develop #2638 +/- ##
===========================================
- Coverage 55.54% 54.56% -0.98%
===========================================
Files 400 399 -1
Lines 26609 25596 -1013
Branches 1242 1242
===========================================
- Hits 14780 13967 -813
+ Misses 11829 11629 -200
Continue to review full report at Codecov.
|
I noticed this morning that although we have the ability to relaunch
VisualizationToolsfrom the UI, if one was to share the url of a runningVisualizationToolwith someone else who accesses said url after saidVisualizationTool'scontainer had been purged then they would get the "Please Wait" page indefinitely.This PR relaunches
VisualizationToolsif they aren't already running when their url is accessed