Remove useless explicit LoadingOption param
#355
Conversation
GlassOfWhiskey
changed the title
LoadingOption param
This commit removes the useless propagation of the `LoadingOptions` object when calling the `load_step` method. This fix also improves performance when parsing large workflows.
GlassOfWhiskey
force-pushed
the
improve-static-checker
branch
from
4133226 to
e3547ce
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #355 +/- ##
==========================================
- Coverage 34.03% 34.01% -0.02%
==========================================
Files 30 30
Lines 35237 35236 -1
Branches 9470 9470
==========================================
- Hits 11993 11986 -7
- Misses 20599 20602 +3
- Partials 2645 2648 +3 ☔ View full report in Codecov by Sentry. |
This commit, combined with common-workflow-language/cwl-utils#355, improves CWL parsing performance by removing useless propagation of the `LoadingOptions` object when parsing child CWL documents.
There was a problem hiding this comment.
Hey @GlassOfWhiskey, this PR requires making a new fetcher object instead of passing along the existing one, which would break loading for users who added a S3-aware fetcher, for example. I do agree that some of the LoadingOptions (namespaces, schemas, original_doc, addl_metadata, imports, includes, and container) would not be relevant (and may even be harmful) for loading a CWL Process from a URI. no_link_check should be propagated; and it is likely that sharing/propagating the idx (a cache) would also be good.
This commit removes the useless propagation of the
LoadingOptionsobject when calling theload_stepmethod. This fix also improves performance when parsing large workflows.