search

ome / omero-cli-transfer

An OMERO CLI plugin for creating transfer packets between OMERO servers.
https://pypi.org/project/omero-cli-transfer/
GNU General Public License v2.0
16 stars 14 forks source link

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

Closed glyg closed 10 months ago

glyg commented 11 months ago

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.

erickmartins commented 11 months ago

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

joshmoore commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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

git clone git@github.com: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 commented 10 months ago

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

erickmartins commented 10 months ago

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 commented 10 months ago

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

glyg commented 10 months ago

Yes sorry, rebasing now

jburel commented 10 months ago

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

glyg commented 10 months ago

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