diff --git a/.github/workflows/omero_plugin.yml b/.github/workflows/omero_plugin.yml index 4fd379f..e6ea25d 100644 --- a/.github/workflows/omero_plugin.yml +++ b/.github/workflows/omero_plugin.yml @@ -36,5 +36,7 @@ jobs: repository: ome/omero-test-infra path: .omero ref: ${{ secrets.OMERO_TEST_INFRA_REF }} - - name: Build and run OMERO tests + - name: Build and run standard tests run: POLICY_BINARY_ACCESS=+read,+write,+image,+plate .omero/docker $STAGE + - name: Build and run no-plate tests + run: .omero/docker $STAGE diff --git a/.omero b/.omero index 3824e85..a0148a6 160000 --- a/.omero +++ b/.omero @@ -1 +1 @@ -Subproject commit 3824e8568d3eb1352eca3a413811b7d665fb3aef +Subproject commit a0148a60126315b011e74c4d28826d63610e4c97 diff --git a/.omeroci/cli-build b/.omeroci/cli-build index 8ef8e37..9702c00 100755 --- a/.omeroci/cli-build +++ b/.omeroci/cli-build @@ -8,6 +8,8 @@ set -x TARGET=${TARGET:-..} cd $TARGET +BIN_ACCESS=$(/opt/omero/server/venv3/bin/omero config get omero.policy.binary_access) + GUESS=${PWD#*omero-cli-*} PLUGIN=${PLUGIN:-$GUESS} @@ -22,5 +24,8 @@ conda activate omero export OMERO_DIST=${OMERO_DIST:-/opt/omero/server/OMERO.server} omero $PLUGIN -h - -python setup.py test -t test -i ${OMERO_DIST}/etc/ice.config -vs +if [[ $BIN_ACCESS =~ "+plate" ]]; then + python setup.py test -t test -i ${OMERO_DIST}/etc/ice.config -vs -m "not limit_plate" +else + python setup.py test -t test -i ${OMERO_DIST}/etc/ice.config -vs -m limit_plate +fi diff --git a/README.md b/README.md index 27a5644..85dbc0c 100755 --- a/README.md +++ b/README.md @@ -70,6 +70,10 @@ Default is `all` and will create the archive. With `none`, only the `transfer.xm file is created, in which case the last cli argument is the path where the `transfer.xml` file will be written. +`--ignore_errors` gnores any download/export errors during the pack process, +often the result of servers which do not allow Plate downloads (but will +ignore any non-zero return code from `omero download` or `omero export`). + Examples: ``` diff --git a/requirements.txt b/requirements.txt index 3e309a8..889cb55 100755 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ -ezomero>=2.0.0 -ome-types>=0.4.2 +ezomero>=2.0.0<3.0.0 +ome-types>=0.5.0<0.6.0 setuptools>=58.0.0 \ No newline at end of file diff --git a/setup.py b/setup.py index 7ce2dde..e0812ae 100755 --- a/setup.py +++ b/setup.py @@ -84,7 +84,7 @@ def read(fname): packages=['', 'omero.plugins'], package_dir={"": "src"}, name="omero-cli-transfer", - version='1.0.1', + version='1.1.0', maintainer="Erick Ratamero", maintainer_email="erick.ratamero@jax.org", description=("A set of utilities for exporting a transfer" @@ -96,7 +96,7 @@ def read(fname): url="https://github.com/TheJacksonLaboratory/omero-cli-transfer", install_requires=[ 'ezomero>=2.1.0, <3.0.0', - 'ome-types>=0.5.0,<0.6.0' + 'ome-types==0.5.1.post1' ], extras_require={ "rocrate": ["rocrate==0.7.0"], diff --git a/src/omero_cli_transfer.py b/src/omero_cli_transfer.py index f5c55f8..4d87491 100644 --- a/src/omero_cli_transfer.py +++ b/src/omero_cli_transfer.py @@ -32,7 +32,7 @@ from omero.sys import Parameters from omero.rtypes import rstring from omero.cli import CLI, GraphControl -from omero.cli import ProxyStringType +from omero.cli import ProxyStringType, NonZeroReturnCode from omero.gateway import BlitzGateway from omero.model import Image, Dataset, Project, Plate, Screen from omero.grid import ManagedRepositoryPrx as MRepo @@ -223,6 +223,10 @@ def _configure(self, parser): pack.add_argument( "--simple", help="Pack into a human-readable package file", action="store_true") + pack.add_argument( + "--ignore_errors", help="Ignores any download/export errors " + "during the pack process", + action="store_true") pack.add_argument( "--metadata", choices=['all', 'none', 'img_id', 'timestamp', @@ -309,7 +313,7 @@ def _get_path_to_repo(self) -> List[str]: return mrepos def _copy_files(self, id_list: Dict[str, Any], folder: str, - conn: BlitzGateway): + ignore_errors: bool, conn: BlitzGateway): if not isinstance(id_list, dict): raise TypeError("id_list must be a dict") if not all(isinstance(item, str) for item in id_list.keys()): @@ -336,10 +340,34 @@ def _copy_files(self, id_list: Dict[str, Any], folder: str, if rel_path == "pixel_images" or fileset is None: filepath = str(Path(subfolder) / (str(clean_id) + ".tiff")) - cli.invoke(['export', '--file', filepath, id]) + if not ignore_errors: + try: + cli.invoke(['export', '--file', filepath, id], + strict=True) + except NonZeroReturnCode: + print("A file could not be exported - this is " + "generally due to a server not allowing" + " binary downloads.") + shutil.rmtree(folder) + raise NonZeroReturnCode(1, "Download not \ + allowed") + else: + cli.invoke(['export', '--file', filepath, id]) downloaded_ids.append(id) else: - cli.invoke(['download', id, subfolder]) + if not ignore_errors: + try: + cli.invoke(['download', id, subfolder], + strict=True) + except NonZeroReturnCode: + print("A file could not be downloaded - this " + "is generally due to a server not " + "allowing binary downloads.") + shutil.rmtree(folder) + raise NonZeroReturnCode(1, "Download not \ + allowed") + else: + cli.invoke(['download', id, subfolder]) for fs_image in fileset.copyImages(): downloaded_ids.append(fs_image.getId()) else: @@ -349,7 +377,17 @@ def _copy_files(self, id_list: Dict[str, Any], folder: str, ann_folder = str(Path(subfolder).parent) os.makedirs(ann_folder, mode=DIR_PERM, exist_ok=True) id = "File" + id - cli.invoke(['download', id, subfolder]) + if not ignore_errors: + try: + cli.invoke(['download', id, subfolder], strict=True) + except NonZeroReturnCode: + print("A file could not be downloaded - this is " + "generally due to a server not allowing" + " binary downloads.") + shutil.rmtree(folder) + raise NonZeroReturnCode(1, "Download not allowed") + else: + cli.invoke(['download', id, subfolder]) def _package_files(self, tar_path: str, zip: bool, folder: str): if zip: @@ -466,10 +504,10 @@ def __pack(self, args): args.barchive, args.simple, args.figure, self.metadata) - if args.binaries == "all": print("Starting file copy...") - self._copy_files(path_id_dict, folder, self.gateway) + self._copy_files(path_id_dict, folder, args.ignore_errors, + self.gateway) if args.simple: self._fix_pixels_image_simple(ome, folder, md_fp) diff --git a/test/integration/test_transfer.py b/test/integration/test_transfer.py index 459baca..68beb92 100644 --- a/test/integration/test_transfer.py +++ b/test/integration/test_transfer.py @@ -8,6 +8,7 @@ from omero_cli_transfer import TransferControl from cli import CLITest from omero.gateway import BlitzGateway +from omero.cli import NonZeroReturnCode import ezomero import pytest @@ -17,6 +18,9 @@ SUPPORTED = [ "idonly", "imageid", "datasetid", "projectid", "plateid", "screenid"] +PLATESONLY = [ + "plateid", "screenid"] + TEST_FILES = [ "test/data/valid_single_image.tar", "test/data/valid_single_image.zip", @@ -149,6 +153,28 @@ def test_pack(self, target_name, tmpdir): self.cli.invoke(args, strict=True) self.delete_all() + @pytest.mark.parametrize('target_name', sorted(PLATESONLY)) + @pytest.mark.limit_plate + def test_pack_noplate(self, target_name, tmpdir): + self.create_plate(target_name=target_name) + target = getattr(self, target_name) + args = self.args + ["pack", target, str(tmpdir / 'test.tar')] + with pytest.raises(NonZeroReturnCode): + self.cli.invoke(args, strict=True) + assert not (os.path.exists(str(tmpdir / 'test.tar'))) + assert not (os.path.exists(str(tmpdir / 'test.tar_folder'))) + args = self.args + ["pack", "--binaries", "none", target, + str(tmpdir / 'test.tar')] + self.cli.invoke(args, strict=True) + assert os.path.exists(str(tmpdir / "test" / "transfer.xml")) + assert os.path.getsize(str(tmpdir / "test" / "transfer.xml")) > 0 + args = self.args + ["pack", "--ignore_errors", target, + str(tmpdir / 'test_ignore.tar')] + self.cli.invoke(args, strict=True) + assert os.path.exists(str(tmpdir / 'test_ignore.tar')) + assert os.path.getsize(str(tmpdir / 'test_ignore.tar')) > 0 + self.delete_all() + @pytest.mark.parametrize('target_name', sorted(SUPPORTED)) def test_pack_special(self, target_name, tmpdir): if target_name == "datasetid" or target_name == "projectid" or\