Add transfer mode support to RemoteAttachments. #1390
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
valentin-pinkau
changed the title
valentin-pinkau
changed the title
valentin-pinkau
changed the title
fm3
left a comment
fm3
left a comment
There was a problem hiding this comment.
Looking good! Just some small suggestions.
Also, please add a changelog entry :)
|
|
||
| @pytest.mark.skip( | ||
| reason="This won't work in CI as the paths stored in cassettes are always absolute and dependent on the system recording the cassette." | ||
| ) |
There was a problem hiding this comment.
Does that mean the test is never run, unless explicitly selected by a user?
Could it run in the nightly? There are no cassettes used there, right?
There was a problem hiding this comment.
Here we would need cassettes as we need the response from wk where to upload the attachment. So this is very similar to the dataset.upload case. @normanrz suggested to setup a minio and configure the wk to use it as storage. This way, we have the same paths in the CI and locally. This should be done in the near future.
| "Expecting all mags from original dataset and new downsampled mag" | ||
| ) | ||
| assert ( | ||
| assert str( |
There was a problem hiding this comment.
why stringify the paths for the comparison?
There was a problem hiding this comment.
because the ds_path / "segmentation" / "agglomerates" / "agglomerate_view.hdf5" becomes a (((S3Path('testoutput/zarr3_dataset_original', protocol='s3') / 'segmentation') / 'agglomerates') / 'agglomerate_view.hdf5') which is not equal to S3Path('testoutput/zarr3_dataset_original/segmentation/agglomerates/agglomerate_view.hdf5', protocol='s3')
There was a problem hiding this comment.
Fair enough. I mean, maybe it should be equal, though? Is this something we control? But anyway, not as part of this PR
webknossos/webknossos/dataset/layer/segmentation_layer/attachments/attachments.py
Show resolved
Hide resolved
Co-authored-by: Florian M <[email protected]>
…ebknossos-libs into more_transfer_mode_usage
Description:
Todos:
Make sure to delete unnecessary points or to check all before merging: