adds a --metadata-only option to pack #72

glyg · 2023-12-05T17:36:09Z

Here is a PR related to #69

Sorry for the messy diff, I forgot to disable automated black formatting in my editor. I'll try to revert that and just show the additions tomorrow.

Mainly I just changed from the line 494 to the end of the function

I added a very naïve test, but I did not yet manage to have the test suite running.

glyg

I highlighted the relevant changes

glyg · 2023-12-05T17:37:01Z

README.md

+`--server` creates the transfer.xml file but does not copy data
+or generate an archive. The last cli argument is the path where the `transfer.xml`
+file will be written
+


new documentation in README

glyg · 2023-12-05T17:37:11Z

README.md


 Examples:
 ```
 omero transfer pack Image:123 transfer_pack.tar
 omero transfer pack --zip Image:123 transfer_pack.zip
 omero transfer pack Dataset:1111 /home/user/new_folder/new_pack.tar
 omero transfer pack 999 tarfile.tar  # equivalent to Project:999
+omero transfer pack --server Dataset:1111 /home/user/new_folder


new example

glyg · 2023-12-05T17:37:43Z

src/omero_cli_transfer.py

+        pack.add_argument(
+            "--server",
+            help="Only generate the xml file, don't create the archive",


argument added

glyg · 2023-12-05T17:38:45Z

src/omero_cli_transfer.py

+        if args.server and args.simple:
+            raise ValueError("The --server and --simple options are incompatible")
+


--server and --simple are mutually exclusive (I think?)

glyg · 2023-12-05T17:39:32Z

src/omero_cli_transfer.py

+        if not args.server:
+            tar_path = Path(args.filepath)
+            folder = str(tar_path) + "_folder"
+        else:
+            tar_path = Path(args.filepath)
+            if tar_path.suffix:
+                folder = os.path.splitext(tar_path)[0]
+                print("Output will be written to {folder}")
+            else:
+                folder = tar_path


filepath argument change

glyg · 2023-12-05T17:39:54Z

src/omero_cli_transfer.py

+        if not args.server:
+            print("Starting file copy...")
+            self._copy_files(path_id_dict, folder, self.gateway)
+


glyg · 2023-12-05T17:40:07Z

src/omero_cli_transfer.py

+        elif not args.server:
+            self._package_files(os.path.splitext(tar_path)[0], args.zip, folder)


don't pacakge

erickmartins · 2023-12-06T14:25:01Z

That was fast! I have a busy day today but will have a closer look tomorrow. Thanks a lot!

joshmoore · 2023-12-06T14:50:40Z

README.md

-`--metadata` allows you to specify which transfer metadata will be saved in `transfer.xml` as possible MapAnnotation values to the images. Defaults to image ID, timestamp, software version, source hostname, md5, source username, source group. 
+`--metadata` allows you to specify which transfer metadata will be saved in `transfer.xml` as possible MapAnnotation values to the images. Defaults to image ID, timestamp, software version, source hostname, md5, source username, source group.
+
+`--metadata_only` creates the transfer.xml file but does not copy data


Looking at this parallelism, I wonder if this is actually a --binary flag similar to the --metadata flag which takes options: files (default), none, and then in the future we could ask it to convert to OME-TIFF/OME-Zarr, for example. That doesn't help with the final argument handling or the exclusivity with simple/rocrate/etc.

Here "binary" means all data that is not metadata?

Generally, "big things that might be downloaded". File attachments might end up being a special case with my other examples (OME-TIFF/Zarr). This is certainly where --server was more straight-forward because it meant, "I have access to all of the binaries and can do what I want with them"

at some point if we decide to be granular about which kinds of annotations are saved with the images as well. maybe 3 similar options covering what can be stored (--metadata, --annotations, --binaries) is the way to go?

I can see that, with there just being some confusing overlap between metadata and annotation (and in the case of FileAnnotations, all three)

Maybe the confusion is lessened if this is ordered metadata < annotation < full metadata would then be a single file (xml, tsv, jsonld) and annotation everything but the binaries.

I don't see why you would want only annotations or binaries

IDK where ROIs sit though

I think this will probably need some rework in the near future anyway (current --metadata would probably be better as --provenance or something like that) and there are good questions (ROIs? FileAnnotations? Where do they fit?). But in general I like --binaries as a solution for now ("none" would fall into the case for this PR, "all" would be default).

joshmoore · 2024-01-03T15:31:25Z

A minor heads up that my test of:

omero transfer pack --binaries=none Image:42665 transfer.tar

was slightly confusing in that it didn't tar but created the "transfer" directory.

glyg · 2024-01-05T10:30:49Z

Yes, this is not ideal! Maybe in that case (if the last positional argument ends with .tar a single file archive should be created?

erickmartins · 2024-01-08T16:07:31Z

Yes, this is not ideal! Maybe in that case (if the last positional argument ends with .tar a single file archive should be created?

that makes sense to me. (in fact, this should probably be true for everything: if it ends in .tar it generates a tar file, if it ends in .zip it generates a zip, if it's a folder path it creates that folder (with relevant files inside). Future improvements!

glyg · 2024-01-23T14:31:37Z

Hi @erickmartins I can't find how to re-run the tests, it does not look like something I can fix on my side.

Is there anything I should do now to advance the PR? I can implement what @joshmoore was saying about argument parsing

joshmoore · 2024-01-23T15:00:41Z

Assuming #75 is merged in or this is rebased on top, then I'd assume:

git clone [email protected]:ome/omero-test-infra .omero
.omero/docker cli

should run the tests. (@jburel can let us know if a special branch of omero-test-infra is needed)

jburel · 2024-01-23T15:34:27Z

the tests are now passing in #75.
When #75 is merged, this PR should be green after a rebase

erickmartins · 2024-01-23T19:52:10Z

#75 is now merged. Note that you do need to set an environment variable with POLICY_BINARY_ACCESS=+read,+write,+image,+plate to get tests to pass locally.

jburel · 2024-01-24T09:08:21Z

@glyg you need to rebase, the files have now been removed. This repository uses the upstream omero-test-infra

glyg · 2024-01-24T09:09:49Z

Yes sorry, rebasing now

jburel · 2024-01-24T15:51:32Z

strange that f073695 is necessary I will have assumed that this should be included with the rebase

glyg · 2024-01-25T10:14:11Z

might be my bad, I solved a lot of merge conflict, maybe I wrongly rm'ed .omero at some point

glyg commented Dec 5, 2023

View reviewed changes

glyg changed the title ~~adds a --server option to pack~~ adds a --metadata-only option to pack Dec 6, 2023

joshmoore reviewed Dec 6, 2023

View reviewed changes

glyg added 14 commits January 24, 2024 10:11

adds a --server option to pack

0a9b122

removed black autoformatting

5ddae9f

fix too many blank lines

e2095be

changes cli option to avoid collision

e9fb550

line length

a7653b1

adds store_true to add_argument

e5e4593

rename tests

4339f66

changes cli argument to

dd5f530

fix line lengths

1045082

removes

fe3ab57

wrong valuerror test

ccb1cad

line length (sorry)

cf3bf83

fix readme (doc was on the wrong section)

d2dff35

update .omero, some test failing

86c17b1

glyg force-pushed the server branch from be5209a to 86c17b1 Compare January 24, 2024 09:16

remove spurious merge chars, without ruff formating

335464d

glyg force-pushed the server branch from cc20afa to 335464d Compare January 24, 2024 09:38

glyg and others added 3 commits January 24, 2024 10:53

merge chores

23e6427

restore wrongly deleted lines

96f2f9b

making sure submodule is still there

f073695

erickmartins merged commit 97cf7ef into ome:main Jan 25, 2024
1 check passed

glyg deleted the server branch January 25, 2024 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adds a --metadata-only option to pack #72

adds a --metadata-only option to pack #72

glyg commented Dec 5, 2023

glyg left a comment

glyg Dec 5, 2023

glyg Dec 5, 2023

glyg Dec 5, 2023

glyg Dec 5, 2023

glyg Dec 5, 2023

glyg Dec 5, 2023

glyg Dec 5, 2023

erickmartins commented Dec 6, 2023

joshmoore Dec 6, 2023

glyg Dec 6, 2023

joshmoore Dec 6, 2023

erickmartins Dec 6, 2023 •

edited

Loading

joshmoore Dec 6, 2023

glyg Dec 7, 2023

glyg Dec 7, 2023

erickmartins Dec 8, 2023

joshmoore commented Jan 3, 2024

glyg commented Jan 5, 2024

erickmartins commented Jan 8, 2024

glyg commented Jan 23, 2024

joshmoore commented Jan 23, 2024

jburel commented Jan 23, 2024

erickmartins commented Jan 23, 2024

jburel commented Jan 24, 2024

glyg commented Jan 24, 2024

jburel commented Jan 24, 2024

glyg commented Jan 25, 2024

		if args.server and args.simple:
		raise ValueError("The --server and --simple options are incompatible")

		elif not args.server:
		self._package_files(os.path.splitext(tar_path)[0], args.zip, folder)

adds a --metadata-only option to pack #72

adds a --metadata-only option to pack #72

Conversation

glyg commented Dec 5, 2023

glyg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erickmartins commented Dec 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erickmartins Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshmoore commented Jan 3, 2024

glyg commented Jan 5, 2024

erickmartins commented Jan 8, 2024

glyg commented Jan 23, 2024

joshmoore commented Jan 23, 2024

jburel commented Jan 23, 2024

erickmartins commented Jan 23, 2024

jburel commented Jan 24, 2024

glyg commented Jan 24, 2024

jburel commented Jan 24, 2024

glyg commented Jan 25, 2024

erickmartins Dec 6, 2023 •

edited

Loading