Fix duplicate download (#450) #477
Conversation
jessebrennan
force-pushed
the
issues/jessebrennan/450-dup-download
branch
2 times, most recently
from
e7af521
to
efeac6d
Compare
Codecov Report
@@ Coverage Diff @@
## master #477 +/- ##
==========================================
+ Coverage 85.54% 85.67% +0.13%
==========================================
Files 42 42
Lines 1895 1906 +11
==========================================
+ Hits 1621 1633 +12
+ Misses 274 273 -1
Continue to review full report at Codecov.
|
jessebrennan
force-pushed
the
issues/jessebrennan/450-dup-download
branch
2 times, most recently
from
67ef538
to
4e3ef4e
Compare
|
@hannes-ucsc tests were failing due to a flaky test. An issue has been created #490 |
jessebrennan
force-pushed
the
issues/jessebrennan/450-dup-download
branch
from
2d0c1c1
to
60520a4
Compare
jessebrennan
force-pushed
the
issues/jessebrennan/450-dup-download
branch
from
60520a4
to
d4d48ed
Compare
|
@hannes-ucsc in order to facilitate resolution of merge conflicts during rebasing, I had to squash the PR. I kept the review changes from your latest review as separate fixup! commits. I also added a couple new commits to address some broken tests. |
| @@ -17,5 +23,34 @@ def test_dict_merge(self): | |||
| dict3 = {'a': {'b': {'c': 1, 'd': 1}}, 'c': 2, 'd': 2} | |||
| self.assertEqual(hca.util._merge_dict(dict1, dict2), dict3) | |||
|
|
|||
|
|
|||
| class TestLinking(TmpDirTestCase): | |||
| """TmpDirTestCase will ensure any links / files we create are cleaned up""" | |||
There was a problem hiding this comment.
I don't like one-liner docstrings because I like the vertical space introduced by the two lines with just triple quotes on them. But there is no clear guide for this repo so I'll leave this to you.
There was a problem hiding this comment.
Since this format of docstring is still PEP 8, I think it is acceptable to leave it as is.
|
Merge at will. |
We want downloads to not overwrite files in the filestore, otherwise we risk overwriting a copy that is already linked elsewhere and thus creating a redundant copy from the old link. Instead, fail to write, but don't complain. This insures we use the filestore copy that already exists.
There's no reason not to set it, and it makes debugging some failures easier.
jessebrennan
force-pushed
the
issues/jessebrennan/450-dup-download
branch
from
d4d48ed
to
8c4b0f2
Compare
Fixes race in file download where the file in the filestore could be overwritten, which would leave a dangling link to the file outside the filestore and thus a duplicate file.