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

Scottx611x/workflow launching and monitoring #1949

Merged
merged 54 commits into from Aug 1, 2017

Conversation

scottx611x
Copy link
Member

@scottx611x scottx611x commented Jul 28, 2017

  • Add Tool Polymorphism (there are now WorkflowTool & VisualizationTool)
  • Provide monitoring for RefineryImport, GalaxyImport, GalaxyAnalysis stages for WorkflowTool launches
  • Create <DatasetCollection:list>, Invoke WorkflowTools with said DatasetCollection, and have the GalaxyAnalysis stage successfully complete.
  • Add analysis_manager.tasks._check_galaxy_history_state for easier testing

NOTE This PR includes extensive test coverage for our workflow launching code within analysis_manager.tests & tool_manager.tests and those tests alone add 500 lines to this diff

@codecov-io
Copy link

codecov-io commented Jul 28, 2017

Codecov Report

Merging #1949 into develop will increase coverage by 1.31%.
The diff coverage is 97.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #1949      +/-   ##
==========================================
+ Coverage    42.78%   44.1%   +1.31%     
==========================================
  Files          414     415       +1     
  Lines        26988   28087    +1099     
  Branches      1321    1322       +1     
==========================================
+ Hits         11548   12388     +840     
- Misses       15440   15699     +259
Impacted Files Coverage Δ
refinery/core/tests.py 99.62% <100%> (ø) ⬆️
refinery/analysis_manager/models.py 82.19% <100%> (+5.52%) ⬆️
refinery/tool_manager/tests.py 99.76% <100%> (+0.05%) ⬆️
refinery/tool_manager/utils.py 85.93% <100%> (+0.61%) ⬆️
refinery/tool_manager/views.py 95.34% <100%> (+0.11%) ⬆️
refinery/tool_manager/test_data/galaxy_mocks.py 100% <100%> (ø) ⬆️
refinery/factory_boy/django_model_factories.py 100% <100%> (ø) ⬆️
refinery/tool_manager/models.py 92.93% <89.1%> (-1.75%) ⬇️
refinery/analysis_manager/tests.py 97.97% <97.46%> (-0.33%) ⬇️
refinery/analysis_manager/tasks.py 48.09% <98.79%> (+21.53%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9f4100...a47ddfc. Read the comment docs.

).delete()
galaxy_workflow_taskset.delete()
analysis.galaxy_cleanup()
return
Copy link
Member Author

Choose a reason for hiding this comment

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

Why these blank returns throughout analysis monitoring code?

@@ -352,13 +347,58 @@ def _get_analysis_config(self):
}

def launch(self):
if self.get_tool_type() == ToolDefinition.VISUALIZATION:
return self._launch_visualization()
raise NotImplementedError(Tool.LAUNCH_WARNING)
Copy link
Member Author

Choose a reason for hiding this comment

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

Look at AbstractBaseClass decorators: @abstractmethod
https://docs.python.org/2/library/abc.html

'src': 'hda'
}
for item in self.get_file_relationships_galaxy()
]
Copy link
Member Author

Choose a reason for hiding this comment

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

style ]

'name': "{} File: {}".format(self.name, item),
'src': 'hda'
}
for item in self.get_file_relationships_galaxy()
Copy link
Member Author

Choose a reason for hiding this comment

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

rename item to something better

structure=structure
)
except RuntimeError:
return structure[:-1]
Copy link
Member Author

Choose a reason for hiding this comment

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

This line on 559 and no runtimeException handling

inputs=self.create_workflow_inputs()
)

def _get_nesting_string(self, nesting=None, structure=""):
Copy link
Member Author

Choose a reason for hiding this comment

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

flatten structure into array and do list/paired concat. from that DS

tool_launch_config[self.FILE_RELATIONSHIPS_GALAXY] = (
tool_launch_config[self.FILE_RELATIONSHIPS]
)
if tool_launch_config.get(self.FILE_RELATIONSHIPS_GALAXY) is None:
Copy link
Member Author

Choose a reason for hiding this comment

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

Set this in create_tool()

if tool_type == ToolDefinition.WORKFLOW:
tool_launch_configuration[WorkflowTool.GALAXY_DATA] = {}
tool = WorkflowToolFactory(
name=tool_name,
Copy link
Member Author

Choose a reason for hiding this comment

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

DRY

Copy link
Member Author

Choose a reason for hiding this comment

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

user_tools = get_objects_for_user(
self.request.user,
"tool_manager.read_tool"
user_tools.extend(
Copy link
Member Author

Choose a reason for hiding this comment

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

is there a better way?

@scottx611x scottx611x merged commit 0b88aa4 into develop Aug 1, 2017
@scottx611x scottx611x deleted the scottx611x/workflow_launching_and_monitoring branch August 1, 2017 15:05
@scottx611x scottx611x added this to the Release 1.6.0 milestone Aug 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants