add python_paths option to config #43
Conversation
Codecov Report
@@ Coverage Diff @@
## master #43 +/- ##
==========================================
+ Coverage 83.91% 84.24% +0.32%
==========================================
Files 9 10 +1
Lines 1082 1111 +29
==========================================
+ Hits 908 936 +28
- Misses 174 175 +1
Continue to review full report at Codecov.
|
| @@ -123,39 +122,54 @@ def run(pipeline_config_filename, extra_config=None, dry_run=False): | |||
| else: | |||
| raise ValueError("Unknown pipeline launcher {launcher_name}") | |||
|
|
|||
| # Run the pre-script. Since it's an error for this to fail (because | |||
There was a problem hiding this comment.
Almost all of this is just an indentation change, to put the main block inside the with statement.
There was a problem hiding this comment.
I've been looking at this for a while today, sorry it took long. So my hesitation with approving is adding paths at the pipeline level. If I read correctly that's what is happening at the pipeline runner stage.
Allowing for custom paths is good, but should typically be done at the stage level rather than the pipeline level. For instance if you have a pipeline runner that farms out tasks to several nodes, it should be at the stage level that we should make sure the paths are correctly set.
So, I'm gonna go ahead and approve it, and open an issue for later to move the PATH definition at the stage level. Unless it's easy do that as part this PR ^^'
|
Thanks - this is useful! I think there are cases for doing this at both levels. At the pipeline level you may want to import pipeline stages from packages in multiple directories to include in the pipeline. This case came up for FlexZPipe. I agree it also worth thinking about passing these to stage level too - will have a think. |
Add feature to temporarily add extra paths to sys.path during pipeline execution, and then remove them afterwards.
Address #42