Skip to content
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

Fix uploading Zarr within a BIDS dataset; typing-check guided fix for handling requests exception #1331

Merged
merged 3 commits into from
Sep 29, 2023

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Sep 28, 2023

could not run tests locally unfortunately . See first commit for more details

Closes #1307

TODOs:

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (697af7c) 88.81% compared to head (f6a9970) 88.85%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1331      +/-   ##
==========================================
+ Coverage   88.81%   88.85%   +0.03%     
==========================================
  Files          76       76              
  Lines       10200    10216      +16     
==========================================
+ Hits         9059     9077      +18     
+ Misses       1141     1139       -2     
Flag Coverage Δ
unittests 88.85% <83.33%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
dandi/files/bids.py 97.50% <100.00%> (ø)
dandi/tests/fixtures.py 97.56% <100.00%> (+0.03%) ⬆️
dandi/tests/test_upload.py 100.00% <100.00%> (ø)
dandi/dandiapi.py 86.74% <0.00%> (-0.24%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…n BIDS

could not run tests locally unfortunately - likely due to my custom
configuration of networking for podman (?) or was it also done for docker? TODO to figure it out

    ulling django   ... done
    Creating network "dandiarchive-docker_default" with the default driver
    ERROR: Failed to Setup IP tables: Unable to enable SKIP DNAT rule:  (iptables failed: iptables --wait -t nat -I DOCKER -i br-c26ebcecb035 -j RETURN: iptables: No chain/target/match by that name.
     (exit status 1))
    Removing network dandiarchive-docker_default
    WARNING: Network dandiarchive-docker_default not found.
    ERROR

    ============================================================ ERRORS =============================================================
    ____________________________________________ ERROR at setup of test_upload_bids_zarr ____________________________________________
    dandi/tests/fixtures.py:403: in docker_compose_setup
        run(
    /usr/lib/python3.11/subprocess.py:571: in run
        raise CalledProcessError(retcode, process.args,
    E   subprocess.CalledProcessError: Command '['docker-compose', 'run', '--rm', 'django', './manage.py', 'migrate']' returned non-zero exit status 1.
    ===================================================== slowest 10 durations ======================================================
    119.18s setup    dandi/tests/test_upload.py::test_upload_bids_zarr

    (1 durations < 0.005s hidden.  Use -vv to show these durations.)
    ==================================================== short test summary info ====================================================
    ERROR dandi/tests/test_upload.py::test_upload_bids_zarr - subprocess.CalledProcessError: Command '['docker-compose', 'run', '--rm', 'django', './manage.py', 'migrate']' returned non-...
    ================================================= 1 error in 119.31s (0:01:59) ==================================================
    python -m pytest -s -v dandi/tests/test_upload.py::test_upload_bids_zarr  3.87s user 3.43s system 6% cpu 2:01.11 total

Also flake8 was not happy either

    flake8...................................................................Failed
    - hook id: flake8
    - exit code: 1

    dandi/tests/test_upload.py:297:5: F821 undefined name 'new_dandiset'
    dandi/tests/test_upload.py:298:16: F821 undefined name 'new_dandiset'
    dandi/tests/test_upload.py:303:5: F821 undefined name 'new_dandiset'

so it seems it does not know how to deal with those pytest fixtures but odd
that just now and just for that one
@yarikoptic
Copy link
Member Author

issue reproduced!
____________________________ test_upload_bids_zarr _____________________________
dandi/tests/test_upload.py:297: in test_upload_bids_zarr
    bids_zarr_dandiset.upload()
dandi/tests/fixtures.py:516: in upload
    upload(
dandi/upload.py:339: in upload
    for v in process_path(dfile):
dandi/upload.py:264: in process_path
    for r in dfile.iter_upload(
dandi/files/bases.py:339: in iter_upload
    etagger = get_dandietag(self.filepath)
/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/fscacher/cache.py:165: in fingerprinter
    ret = fingerprinted(*args, **kwargs_)
/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/joblib/memory.py:655: in __call__
    return self._cached_call(args, kwargs)[0]
/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/joblib/memory.py:598: in _cached_call
    out, metadata = self.call(*args, **kwargs)
/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/joblib/memory.py:856: in call
    output = self.func(*args, **kwargs)
/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/fscacher/cache.py:101: in fingerprinted
    return f(path, *args, **kwargs)
dandi/support/digests.py:100: in get_dandietag
    return DandiETag.from_file(filepath)
/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/dandischema/digests/dandietag.py:147: in from_file
    with open(path, "rb") as f:
E   IsADirectoryError: [Errno 21] Is a directory: '/tmp/pytest-of-runner/pytest-0/dandiset236/sub-01/ses-01/micr/sub-01_ses-01_sample-A_SPIM.ome.zarr'

…Zarr is used not of a file

Also fixed up test
@yarikoptic yarikoptic changed the title Add a test checking/demonstrating the problem of uploading Zarr within BIDS Fix uploading Zarr within a BIDS dataset Sep 29, 2023
@yarikoptic yarikoptic added the patch Increment the patch version when merged label Sep 29, 2023
@yarikoptic yarikoptic marked this pull request as ready for review September 29, 2023 00:42
@yarikoptic yarikoptic changed the title Fix uploading Zarr within a BIDS dataset Fix uploading Zarr within a BIDS dataset; typing-check guided fix for handling requests exception Sep 29, 2023
@yarikoptic yarikoptic added the release Create a release when this pr is merged label Sep 29, 2023
@yarikoptic yarikoptic merged commit cb984e8 into master Sep 29, 2023
23 of 24 checks passed
@yarikoptic yarikoptic deleted the bf-1307 branch September 29, 2023 03:05
@github-actions
Copy link

🚀 PR was released in 0.56.2 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot upload zarr files
1 participant