Skip to content

Replace Minio dependency with a service#39

Merged
sbesson merged 7 commits intoome:masterfrom
manics-archive:s3-service
Feb 6, 2019
Merged

Replace Minio dependency with a service#39
sbesson merged 7 commits intoome:masterfrom
manics-archive:s3-service

Conversation

@manics
Copy link
Copy Markdown
Member

@manics manics commented Feb 4, 2019

Replaces the Minio client dependency with an optional service. See https://trello.com/c/UJGyYSrg/129-s3-and-imagej

Testing: check you can run bioformats without Minio.

@sbesson sbesson added this to the 6.0.0 milestone Feb 4, 2019
Copy link
Copy Markdown
Member

@melissalinkert melissalinkert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments aside, I don't see any obvious issues reading through this. Leaving @sbesson to conduct functional tests as discussed.

Comment thread src/test/java/loci/common/utests/S3ClientServiceTest.java Outdated
Comment thread src/main/java/loci/common/S3Handle.java
Comment thread src/main/java/loci/common/services/S3ClientService.java
Comment thread src/main/java/loci/common/services/S3ClientStat.java
Comment thread src/main/java/loci/common/services/S3ClientServiceException.java Outdated
@manics
Copy link
Copy Markdown
Member Author

manics commented Feb 5, 2019

@melissalinkert @mtbc Thanks for reviewing, I've updated this PR

@sbesson
Copy link
Copy Markdown
Member

sbesson commented Feb 5, 2019

Initial testing based on the state of 3a59777 looked good. In a Fiji distribution with indivually replaced JARs including the new ome-common, I was able to open individual files of various formats without issue.

As additional commits have been pushed since, in #39 (commits), re-testing will have to be performed with a new build including the last changes. Looks promising.

Copy link
Copy Markdown
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retested using a local Fiji application with manually modified JARs including ome-common-6.0.0-SNAPSHOT with this PR included

screen shot 2019-02-06 at 13 14 41

Opening images from various formats using the Bio-Formats importer works as expected i.e. the error message reported in ome/bio-formats-fiji#19 (comment) is no longer thrown immediately after selecting the file and both metadata and pixel data are read as expected.

I have not tested the workflow with minio included pointing to a remote location on s3 but from a functional perspective, the addition of the service is sufficient to preserve the current statu quo in terms of deployment of the Bio-Formats plugin in the Fiji ecosystem.

Assuming @mtbc and @melissalinkert are also happy with the last couple of changes, this should be ready to merge and release ome-common 6.0.0.

@sbesson sbesson merged commit a08e2d5 into ome:master Feb 6, 2019
@manics manics deleted the s3-service branch February 6, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants