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

[18.01] Fix rename post job actions to be more intuitive when mapping over collections. #5416

Merged

Conversation

Projects
None yet
3 participants
@jmchilton
Copy link
Member

jmchilton commented Jan 30, 2018

Actually rename to the mapped over collection output in addition to the individual datasets. RenameDatasetAction might better be terms RenameOutputAction now but that isn't done in this PR.

Applies #5414 targeting 17.09 to 18.01 also since this modifies that piece of code.

@mvdbeek

This comment has been minimized.

Copy link
Member

mvdbeek commented Jan 31, 2018

Traceback (most recent call last):
  File "/galaxy/lib/galaxy/web/framework/decorators.py", line 281, in decorator
    rval = func(self, trans, *args, **kwargs)
  File "/galaxy/lib/galaxy/webapps/galaxy/api/history_contents.py", line 178, in show
    return self.__show_dataset_collection(trans, id, history_id, **kwd)
  File "/galaxy/lib/galaxy/webapps/galaxy/api/history_contents.py", line 266, in __show_dataset_collection
    dataset_collection_instance = self.__get_accessible_collection(trans, id, history_id)
  File "/galaxy/lib/galaxy/webapps/galaxy/api/history_contents.py", line 277, in __get_accessible_collection
    id=id
  File "/galaxy/lib/galaxy/managers/collections.py", line 376, in get_dataset_collection_instance
    return self.__get_history_collection_instance(trans, id, **kwds)
  File "/galaxy/lib/galaxy/managers/collections.py", line 388, in __get_history_collection_instance
    history = getattr(trans, 'history', collection_instance.history)
AttributeError: 'NoneType' object has no attribute 'history'
FAIL

meh, seems like we have a history_id and then we throw it away.
I think the hdca id was wrong, and then get returns None in

collection_instance = trans.sa_session.query(trans.app.model.HistoryDatasetCollectionAssociation).get(instance_id)

type: File
name: fasta1
""", history_id=history_id)
details1 = self.dataset_populator.get_history_collection_details(history_id, hid=1, wait=True, assert_ok=True)

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Jan 31, 2018

Member

I think hid 1 is the fasta input, but if you don't specify the hid you should get the last one, which should be the collection

This comment has been minimized.

Copy link
@jmchilton

jmchilton Jan 31, 2018

Author Member

I was like why does it work locally? The answer is it works by generating an ID for the hid supplied and then asking for that ID - so I grabbed an HDA I think and got the corresponding ID which was the correct collection maybe? I don't know - something to do with overlapping IDs again I think. I'll work on it - thanks!

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Jan 31, 2018

Member

Yep, the IDs are just the encoded database id integers, so you'd just fetch the collecton with id 1 in a local test, which just works.

@jmchilton jmchilton force-pushed the jmchilton:rename_collections_pja branch 2 times, most recently from 7732fb5 to 954b929 Jan 31, 2018

@mvdbeek

This comment has been minimized.

Copy link
Member

mvdbeek commented Feb 1, 2018

So renaming alone works beautifully, but I guess it would be better to merge this as part of #5418,
since including a tag PJA results in:

Traceback (most recent call last):
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 186, in invoke
    incomplete_or_none = self._invoke_step(workflow_invocation_step)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 262, in _invoke_step
    use_cached_job=self.workflow_invocation.use_cached_job)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/modules.py", line 940, in execute
    self._handle_mapped_over_post_job_actions(step, step_inputs, step_outputs, invocation.replacement_dict)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/modules.py", line 989, in _handle_mapped_over_post_job_actions
    ActionBox.execute_on_mapped_over(self.trans.app, self.trans.sa_session, pja, step_inputs, step_outputs, replacement_dict)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/jobs/actions/post.py", line 450, in execute_on_mapped_over
    ActionBox.actions[pja.action_type].execute_on_mapped_over(app, sa_session, pja, step_inputs, step_outputs, replacement_dict)
AttributeError: type object 'TagDatasetAction' has no attribute 'execute_on_mapped_over'
galaxy.workflow.run ERROR 2018-02-01 09:25:23,012 Failed to execute scheduled workflow.
Traceback (most recent call last):
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 80, in __invoke
    outputs = invoker.invoke()
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 186, in invoke
    incomplete_or_none = self._invoke_step(workflow_invocation_step)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 262, in _invoke_step
    use_cached_job=self.workflow_invocation.use_cached_job)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/modules.py", line 940, in execute
    self._handle_mapped_over_post_job_actions(step, step_inputs, step_outputs, invocation.replacement_dict)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/modules.py", line 989, in _handle_mapped_over_post_job_actions
    ActionBox.execute_on_mapped_over(self.trans.app, self.trans.sa_session, pja, step_inputs, step_outputs, replacement_dict)
  File "/Users/mvandenb/src/galaxy/lib/galaxy/jobs/actions/post.py", line 450, in execute_on_mapped_over
    ActionBox.actions[pja.action_type].execute_on_mapped_over(app, sa_session, pja, step_inputs, step_outputs, replacement_dict)
AttributeError: type object 'TagDatasetAction' has no attribute 'execute_on_mapped_over'

While that didn't work correctly before it now fails to schedule.

@jmchilton

This comment has been minimized.

Copy link
Member Author

jmchilton commented Feb 2, 2018

@mvdbeek Thanks for the not about tagging - I think the problem is I created this mapped_over_output_actions attribute but didn't use it. I'll fix that up here - because that other PR won't fix the problem that the method is still going to be called for other operations that don't make sense to execute on the mapped over collection (e.g. set column or change datatype).

Update: Should be fixed with 0529153 - I'll see if I can make some progress on #5418 today though.

jmchilton added some commits Jan 30, 2018

Apply rename PJA to collection outputs including $ replacements.
Update PJA collection test to test HDCA is renamed also.
Extend mapped collection output rename PJAs to include # replacements.
Also make sure output collections in non-mapping scenarios are also properly renamed as well.

@jmchilton jmchilton force-pushed the jmchilton:rename_collections_pja branch from 954b929 to 0529153 Feb 2, 2018

@mvdbeek mvdbeek merged commit fbfbdd5 into galaxyproject:release_18.01 Feb 2, 2018

6 checks passed

api test Build finished. 344 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 171 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 79 tests run, 4 skipped, 0 failed.
Details
selenium test Build finished. 118 tests run, 3 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@mvdbeek

This comment has been minimized.

Copy link
Member

mvdbeek commented Feb 2, 2018

Wonderful, that (and #5418) is a big usability improvements

@galaxybot

This comment has been minimized.

Copy link

galaxybot commented Feb 2, 2018

This PR was merged without a milestone attached.

@jmchilton jmchilton added this to the 18.01 milestone Feb 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.