Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZarrReader: Update getUsedFiles to use the noPixels flag #41

Closed
wants to merge 1 commit into from

Conversation

dgault
Copy link
Member

@dgault dgault commented Sep 12, 2022

This is an update to the getUsedFiles method to use the noPixels parameter, which before now had been ignored like in many other readers. With this PR calling getUsedFiles with noPixels set to true should return the metadata files for the dataset. For the ZarrReader this will be the zattrs and zgroup files along with any xml files that may also be present.

@will-moore
Copy link
Member

@dgault - Just discussed this PR at IDR meeting: - I realise that this isn't going to work when importing NGFF data into OMERO because the server will not be able to find the chunks after import.

Currently, on import you will get a symlink created under ManagedRepo to e.g. ManagedRepo/path/to/image.zarr/0/[symlink-to-.zarray] but if you look into ManagedRepo/path/to/image.zarr/0/ you won't find a symlink to the chunk.

What we currently do manually (after in-place import) is to replace e.g. ManagedRepo/path/to/image.zarr/ with a symlink to the image.zarr directory of the original data. So then you will find a chunk if you look into ManagedRepo/path/to/[symlink-to-image.zarr]/0/.

But currently we have no logic to create symlinks to directories during in-place import, only symlinks to files.

Also, if we want this to work with non in-place import then we'd still want to copy over all the chunks to the ManagedRepo but NOT create OriginalFiles in OMERO for each chunks.

@joshmoore
Copy link
Member

Closing under the assumption that @will-moore's not currently using this in his workflow.

It's also involved in current conflicts. If testing shows it's needed, I'll re-open.

@joshmoore joshmoore closed this Jul 5, 2023
@joshmoore joshmoore reopened this Jul 6, 2023
@joshmoore
Copy link
Member

So with #57, I now have omero-mkngff working. However, if I omit the chunks (which is desired to not overwhelm the database), then on rendering I see what I assume you were running into, @will-moore:

Caused by: ome.conditions.SecurityViolation: reader for /data/OMERO/ManagedRepository/demo_2/Blitz-0-Ice.ThreadPool.Server-4/2023-07/06/09-55-54.806_converted/a.ome.zarr/OME/METADATA.ome.xml uses file from repository cdf35825-def1-4580-8d0b-9c349b8f78d
6 that is not readable from database
        at ome.services.blitz.repo.ManagedReaderSecurityCheck.assertUsedFilesReadable(ManagedReaderSecurityCheck.java:175)
        at ome.io.nio.PixelsService$3.setId(PixelsService.java:870)

Perhaps, what we actually need is to have noPixels set server-side. (The down-side of this is that delete won't know to delete the chunks, but I propose to let that be something we deal with later.)

client \ server pixels noPixels
pixels (default) ok ok
noPixels throws "not readable" ok with re-symlinking
mkngff throws "not readable" ok with re-symlinking

cc: @sbesson

.forEach(path -> usedFiles.add(path.toFile().getAbsolutePath()));
paths.filter(Files::isRegularFile)
.forEach(path -> {if (!noPixels ||
(noPixels && (path.endsWith(".zgroup") || path.endsWith(".zattrs") || path.endsWith(".xml"))))
Copy link
Member

Choose a reason for hiding this comment

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

Note: for mkngff, the logic I used was "don't enter directories that don't have a .zgroup or .zarray". That might be safer if others start using the "add your own random file" strategy.

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