Add S3Handle#33
Conversation
manics
changed the title
|
Conflicting PR. Removed from build OME-FILES-CPP-DEV-merge-push-superbuild#1069. See the console output for more details.
|
manics
force-pushed
the
s3
branch
2 times, most recently
from
939341f to
206d4db
Compare
|
Rebased on top of current master ( |
URLs starting with `s3://` will now be passed to the S3Handle. `./showinfo s3://bucket/path/file.tiff` then opens the file as expected.
Changes include: - url may now be null even if isURL is not - Location(parent, child) is now the primary constructor - several special cases for s3 at the moment Remaining TODOs: - untangle isDirectory/isFile/exists calls - fix list() - handle http:// similarly to s3:// - add caching for various internal calls like exists() - remove openConnection wherever possible - handle absolute paths (cc: Seb)
In order to allow specifying non-AWS buckets, endpoints and similar information need to be encoded in the URL.
bucket.with.dots.is.indistinguishable.from.server.endpoint
Reduces the amount of magic in S3Handle Also enables S3HandleTest
Include the underlying Minio exception if these was one
It's misleading since we don't know whether it's found or not this early
|
Restarted Travis which was reported false positives. With most of the comments addressed and the decision on |
|
I could add more tests, but that could come now, or in a future PR. I think it's time to test this more heavily to find all related issues in the BF and OMERO stack. |
|
Last commit adds some read/seek S3Handle tests |
|
Last commit caches |
|
@manics: is this ready for a final round of review and integration into a formal 6.0.0 development milestone? |
|
Yes |
| private URI uri; | ||
| private File file; | ||
|
|
||
| class URLLocationProperties { |
There was a problem hiding this comment.
Longer-term, this should likely be at least a static inner class if not a top-level class.
|
👍 on the new commits. We’ll need a review of all the new inner classes in the Location & Handle hierarchies, but happy to do that as a follow on. |
|
Captured all the potential API reviews as https://trello.com/c/DEaOjION/93-ome-common-location-handle. Merging and I will cut a second mileston release of |
|
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/creating-imagereader-without-setid-irods-s3/43807/5 |
|
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/bioformats-image-reader-reading-from-file-a-url-s3/44840/7 |
7 participants
These are the commits from #32 with additional commits on top.
It should be possible to read a file off AWS S3, e.g.
./showinf s3://s3.us-east-1.amazonaws.com/czi.starfish.data.public/browse/raw/20180820/merfish_u2os/fov_57.tifif you don't mind waitingAlso includes primitive caching:
BF_REMOTE_CACHE_ROOTDIR=$HOME/tmp/ BF_DEVEL=true BF_FLAGS="-cp $PWD/../../ome-common-java/target/ome-common-5.3.7-SNAPSHOT.jar:$PWD/bioformats_package.jar:$PWD" ./showinf s3://s3.us-east-1.amazonaws.com/czi.starfish.data.public/browse/raw/20180820/merfish_u2os/fov_57.tifthough it still attempts to check for the existence of many other remote filesExample full timings (including the time taken before the "Initialization" time is calculated) for
s3://s3.us-east-1.amazonaws.com/czi.starfish.data.public/browse/raw/20180820/merfish_u2os/fov_57.tif:Examples using a local minio server with the same file
s3://INTERNAL.HOST/test.bucket/fov_57.tifConclusions so far: