-
Notifications
You must be signed in to change notification settings - Fork 11
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
b2luigi/batch/processes/gbasf2.py: use '--failed_lfns' #132
Conversation
…nloads and remove tests
Codecov Report
@@ Coverage Diff @@
## main #132 +/- ##
==========================================
- Coverage 56.54% 56.30% -0.24%
==========================================
Files 23 23
Lines 1505 1506 +1
==========================================
- Hits 851 848 -3
- Misses 654 658 +4
Continue to review full report at Codecov.
|
Doing "replace" on the file path for the current monitoring file might be problematic, if the strings "failed_files" appears somewhere in the filepath (e.g. a directory). In this case it is unlikely, but still, I prefer using os.path.splitext to get the stem (part before the extension) of a filepath and just append a suffix to that and re-append the extension.
In the past I myself used `open(...).readlines()`, but I found it is considered better style to use `with` to explicitly free resources after opening the file. In the first case the garbage collector would to it at some point. Found this due to a pylint warning, see: https://stackoverflow.com/questions/67419064/r1732-consider-using-with-for-resource-allocating-operations-consider-using
Thanks, I took some time to look at this PR and looks good to me. Small reminder, I still prefer if you could create a separate named feature branch before opening the PR, this makes some git stuff less cumbersome 😉 Thanks to you making this editable I did two minor changes which I thought don't require much discussions:
I just pushed these directly to your |
Hi @meliache thanks a lot for reviewing and fixing the pull request. I totally support your changes :) And sorry for working on my main branch :D bad habbit ;) |
This pull request updates the behaviour of b2luigi in case of failed downloads from gbasf2 tasks and addresses the issue #130.
In the newest gbasf2 release v5r1p3, the option
--failed_lfns
becomes available to pass lfns of failed downloads to a text file. This allows to avoid parsing ofstdout
of the tool, which could lead to errors.Since the format of the file created by
--failed_lfns
is exactly as required by--input_dslist
in the subsequent download, no additional tests are needed. Therefore, all tests related to failed downloads are removed.