Skip to content

metadata v2 v3#17

Merged
lazarusA merged 4 commits into
mainfrom
la/meta_v2_v3
Jun 9, 2026
Merged

metadata v2 v3#17
lazarusA merged 4 commits into
mainfrom
la/meta_v2_v3

Conversation

@lazarusA

@lazarusA lazarusA commented Jun 5, 2026

Copy link
Copy Markdown
Member

we let zarrita handle the metadata requests if is available on disk. And for v2, we generate some if not available. For v3, ideally it should also be handle in zarr.jl, see JuliaIO/Zarr.jl#287

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors metadata handling in src/serve_zarr.jl by replacing _synthetic_zmetadata with helper functions _has_consolidated_metadata and _generate_metadata. However, the review identified three key issues: a critical bug where removing the path check causes the server to return synthetic metadata for any missing file instead of a 404; an inefficiency where Zarr v3 stores trigger unnecessary and expensive metadata generation; and the removal of a guard check that allows arbitrary non-Zarr directories to attempt consolidation. Please address these issues by restoring the path and store-type checks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/serve_zarr.jl Outdated
Comment thread src/serve_zarr.jl
Comment thread src/serve_zarr.jl
@lazarusA

lazarusA commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

\gemini review

@lazarusA lazarusA merged commit 6054f65 into main Jun 9, 2026
9 checks passed
@lazarusA lazarusA deleted the la/meta_v2_v3 branch June 9, 2026 19:47
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.

1 participant