Skip to content

Only parse import uris if stated - #193

Open
dominikl wants to merge 4 commits into
ome:masterfrom
dominikl:http_s3
Open

Only parse import uris if stated#193
dominikl wants to merge 4 commits into
ome:masterfrom
dominikl:http_s3

Conversation

@dominikl

@dominikl dominikl commented Mar 6, 2026

Copy link
Copy Markdown
Member

With ome/omero-zarr-pixel-buffer#13 it won't be necessary any longer to translate http into s3 URLs. The option is still available but only when explicetely stated (via --parseuri flag).

uri = "s3://" + path

if not uri.startswith("/"):
uri = format_s3_uri(uri, endpoint)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Has it been tested with the new format ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If the extinfo URL is http then the format doesn't matter (doesn't care about what part of the URL is the endpoint, bucket, etc).

For the option to convert http url into S3: That would actually need another flag to specify if the provided URL is host or path-style format. Is it worth adding that, as this option is actually not really needed any longer?

@will-moore will-moore left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks fine.
I think it would be useful to document when you don't (or DO) need to --parseuri.
In the argument help it could say "This is required if you are using omero-zarr-pixel-buffer version ??? or earlier". And add a note in the repo README where it describes the "import" option

@snoopycrimecop

Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-plugins-push#52. See the console output for more details.
Possible conflicts:

--conflicts

@dominikl
dominikl requested a review from will-moore August 25, 2026 10:54
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