Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[15.05] Create empty history if history is unavailable during api call #217

Merged
merged 3 commits into from
May 6, 2015

Conversation

guerler
Copy link
Contributor

@guerler guerler commented May 5, 2015

The tool build api shares the basic.py code with the old tool form. The old tool form is only called from the Galaxy UI i.e. a history is always available. In order to prevent an uncaught exception for a plain api call without a valid history, here we create an empty history on the fly. This is a temporary fix which allows users to retrieve a valid tool model if the history has not been created yet. Once the old tool form is disabled (currently composite uploads prevent us from doing that), the history id will be parsed through the api call and we will remove the concept of 'current' history from basic.py too. Until then this fix is required.

@@ -1657,6 +1657,8 @@ def _get_history( self, trans, history=None ):
assert trans is not None, "%s requires a trans" % class_name
if history is None:
history = trans.get_history()
if history is None:
history = trans.get_history( create=True )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a second set of lines, should we just pass create=True to the invocation above?

@carlfeberhard
Copy link
Contributor

+1

I'd second Dannon's suggestion in the line notes.

@dannon
Copy link
Member

dannon commented May 5, 2015

Thanks for the tweak, but apparently this breaks the Galaxy. Digging.

@guerler
Copy link
Contributor Author

guerler commented May 6, 2015

Thanks for the hint Carl. Test cases are fixed.

@dannon
Copy link
Member

dannon commented May 6, 2015

Good call - the reason it passed before is that the tests simply didn't exercise the new code path. Anyway, +1

dannon added a commit that referenced this pull request May 6, 2015
[15.05] Create empty history if history is unavailable during api call
@dannon dannon merged commit bb75068 into galaxyproject:release_15.05 May 6, 2015
@jmchilton
Copy link
Member

I think this breaks the Tools API.

./run_tests.sh -api test/api/test_tools.py:ToolsTestCase.test_show_repeat
Traceback (most recent call last):
  File "/home/john/workspace/galaxy/lib/galaxy/web/framework/decorators.py", line 251, in decorator
    rval = func( self, trans, *args, **kwargs)
  File "/home/john/workspace/galaxy/lib/galaxy/webapps/galaxy/api/tools.py", line 94, in show
    return tool.to_dict( trans, io_details=io_details, link_details=link_details )
  File "/home/john/workspace/galaxy/lib/galaxy/tools/__init__.py", line 2255, in to_dict
    tool_dict[ 'inputs' ] = [ input.to_dict( trans ) for input in self.inputs.values() ]
  File "/home/john/workspace/galaxy/lib/galaxy/tools/parameters/basic.py", line 2079, in to_dict
    d = super( DataToolParameter, self ).to_dict( trans )
  File "/home/john/workspace/galaxy/lib/galaxy/tools/parameters/basic.py", line 211, in to_dict
    tool_dict[ 'html' ] = urllib.quote( util.smart_str( self.get_html( trans ) ) )
  File "/home/john/workspace/galaxy/lib/galaxy/tools/parameters/basic.py", line 96, in get_html
    return self.get_html_field( trans, value, other_values ).get_html()
  File "/home/john/workspace/galaxy/lib/galaxy/tools/parameters/basic.py", line 1741, in get_html_field
    history = self._get_history( trans )
  File "/home/john/workspace/galaxy/lib/galaxy/tools/parameters/basic.py", line 1641, in _get_history
    history = trans.get_history( create=True )
  File "/home/john/workspace/galaxy/lib/galaxy/web/framework/webapp.py", line 674, in get_history
    history = self.new_history()
  File "/home/john/workspace/galaxy/lib/galaxy/web/framework/webapp.py", line 730, in new_history
    self.galaxy_session.current_history = history
AttributeError: 'NoneType' object has no attribute 'current_history'

tools.show shouldn't require a history IMO - it is marked sessionless among other reasons (@expose_api_anonymous_and_sessionless).

@dannon
Copy link
Member

dannon commented May 14, 2015

Yeah, I agree with this. Show shouldn't require context and should just work regardless, while build obviously needs context.

@guerler guerler deleted the fix_missing_history branch May 18, 2015 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants