-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
support plugins for alternative FsAccess #233
Conversation
I copied StdFsAccess and CollectionFsAccess into this project to avoid adding a dependency on cwltool. Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <[email protected]>
Added partial arvados type stubs. Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #233 +/- ##
==========================================
- Coverage 81.52% 81.31% -0.22%
==========================================
Files 8 9 +1
Lines 785 824 +39
Branches 190 195 +5
==========================================
+ Hits 640 670 +30
- Misses 111 118 +7
- Partials 34 36 +2 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
I did all the annotations to make mypy happy as well as linting and running format but codecov is going to block merging, because writing unit tests to cover the Arvados-specific code would only work if I stubbed out everything to the point that they would be useless as tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Peter,
How are you all running cwltest
, using the cwltest
pytest plugin or directly through the cwltest
CLI?
Seems like you want your own plugin to cwltest to use a custom StdFsAccess
class. Perhaps that would be better than duplicating the arvados code here?
Hi Michael, The thing I need is for the file checks to go through a abstracted interface, so I copied in a slimmed down version of If I revise the PR so it just adds that hook, I could remove the Arvados stuff from this PR and instead make my own |
Almost, if |
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <[email protected]>
@mr-c let me know if you're happy with this and we can hit the big green merge button. |
Given the long battle with mypy and the linting tools, it'll probably make sense to squash this one. |
Thank you @tetron ! |
This adds a slimmed down version of
StdFsAccess
class fromcwltool
, which is used to abstract the output file checks.Adds Arvados as an optional dependency. Adjust cwltest to use
ArvadosFsAccess
if thearvados
module is importable.