diff --git a/swh/deposit/cli/client.py b/swh/deposit/cli/client.py --- a/swh/deposit/cli/client.py +++ b/swh/deposit/cli/client.py @@ -129,8 +129,6 @@ archive: Optional[str], metadata: Optional[str], collection: Optional[str], - archive_deposit: bool, - metadata_deposit: bool, slug: Optional[str], partial: bool, deposit_id: Optional[int], @@ -171,33 +169,22 @@ Returns: dict with the following keys: - 'archive': the software archive to deposit - 'username': username - 'metadata': the metadata file to deposit + "archive": the software archive to deposit + "username": username + "metadata": the metadata file to deposit 'collection: the user's collection under which to put the deposit 'slug': the slug or external id identifying the deposit to make - 'partial': if the deposit is partial or not - 'client': instantiated class - 'url': deposit's server main entry point - 'deposit_type': deposit's type (binary, multipart, metadata) - 'deposit_id': optional deposit identifier - 'swhid': optional deposit swhid - + 'in_progress': if the deposit is partial or not + "url": deposit's server main entry point + "deposit_id": optional deposit identifier + "swhid": optional deposit swhid + "replace": whether the given deposit is to be replaced or not """ - if archive_deposit and metadata_deposit: - # too many flags use, remove redundant ones (-> multipart deposit) - archive_deposit = False - metadata_deposit = False if not slug: # generate one as this is mandatory slug = generate_slug() if not metadata: - if metadata_deposit: - raise InputError( - "Metadata deposit must be provided for metadata " - "deposit, either a filepath with --metadata or --name and --author" - ) if name and authors: metadata_path = os.path.join(temp_dir, "metadata.xml") logging.debug("Temporary file: %s", metadata_path) @@ -206,7 +193,7 @@ with open(metadata_path, "w") as f: f.write(metadata_xml) metadata = metadata_path - elif not archive_deposit and not partial and not deposit_id: + elif archive is not None and not partial and not deposit_id: # If we meet all the following conditions: # * this is not an archive-only deposit request # * it is not part of a multipart deposit (either create/update @@ -215,7 +202,6 @@ raise InputError( "For metadata deposit request, either a metadata file with " "--metadata or both --author and --name must be provided. " - "If this is an archive deposit request, none is required." ) elif name or authors: # If we are generating metadata, then all mandatory metadata @@ -234,13 +220,7 @@ "--author and --name (Those are used to generate one metadata file)." ) - if metadata_deposit: - archive = None - - if archive_deposit: - metadata = None - - if not archive and not metadata and partial: + if not archive and not metadata: raise InputError( "Please provide an actionable command. See --help for more information" ) @@ -291,16 +271,6 @@ "this will use a file named .metadata.xml" ), ) # noqa -@click.option( - "--archive-deposit/--no-archive-deposit", - default=False, - help="(Optional) Software archive only deposit", -) -@click.option( - "--metadata-deposit/--no-metadata-deposit", - default=False, - help="(Optional) Metadata only deposit", -) @click.option( "--collection", help="(Optional) User's collection. If not provided, this will be fetched.", @@ -366,8 +336,6 @@ password: str, archive: Optional[str] = None, metadata: Optional[str] = None, - archive_deposit: bool = False, - metadata_deposit: bool = False, collection: Optional[str] = None, slug: Optional[str] = None, partial: bool = False, @@ -404,8 +372,6 @@ archive, metadata, collection, - archive_deposit, - metadata_deposit, slug, partial, deposit_id, diff --git a/swh/deposit/tests/cli/test_client.py b/swh/deposit/tests/cli/test_client.py --- a/swh/deposit/tests/cli/test_client.py +++ b/swh/deposit/tests/cli/test_client.py @@ -249,7 +249,6 @@ "Problem during parsing options: " "For metadata deposit request, either a metadata file with " "--metadata or both --author and --name must be provided. " - "If this is an archive deposit request, none is required." ), ) assert expected_error_log_record in caplog.record_tuples @@ -365,11 +364,14 @@ assert expected_error_log_record in caplog.record_tuples -def test_cli_validation_missing_metadata_flag(caplog, cli_runner): - """--metadata-deposit requires --metadata (or --name and --author) otherwise fails +def test_cli_validation_replace_with_no_deposit_id_fails( + sample_archive, caplog, patched_tmp_path, requests_mock_datadir, datadir, cli_runner +): + """--replace flags require --deposit-id otherwise fails """ - # --metadata-deposit without --metadata flag fails + metadata_path = os.path.join(datadir, "atom", "entry-data-deposit-binary.xml") + result = cli_runner.invoke( cli, [ @@ -380,7 +382,11 @@ TEST_USER["username"], "--password", TEST_USER["password"], - "--metadata-deposit", # should fail because missing --metadata flag + "--metadata", + metadata_path, + "--archive", + sample_archive["path"], + "--replace", ], ) @@ -391,56 +397,12 @@ logging.ERROR, ( "Problem during parsing options: " - "Metadata deposit must be provided for metadata " - "deposit, either a filepath with --metadata or --name and --author" + "To update an existing deposit, you must provide its id" ), ) assert expected_error_log_record in caplog.record_tuples -def test_cli_validation_replace_with_no_deposit_id_fails( - sample_archive, caplog, patched_tmp_path, requests_mock_datadir, datadir, cli_runner -): - """--replace flags require --deposit-id otherwise fails - - """ - metadata_path = os.path.join(datadir, "atom", "entry-data-deposit-binary.xml") - - for flags in [ - ["--replace"], - ["--replace", "--metadata-deposit", "--archive-deposit"], - ]: - result = cli_runner.invoke( - cli, - [ - "upload", - "--url", - "https://deposit.swh.test/1", - "--username", - TEST_USER["username"], - "--password", - TEST_USER["password"], - "--metadata", - metadata_path, - "--archive", - sample_archive["path"], - ] - + flags, - ) - - assert result.exit_code == 1, result.output - assert result.output == "" - expected_error_log_record = ( - "swh.deposit.cli.client", - logging.ERROR, - ( - "Problem during parsing options: " - "To update an existing deposit, you must provide its id" - ), - ) - assert expected_error_log_record in caplog.record_tuples - - def test_cli_single_deposit_slug_generation( sample_archive, patched_tmp_path, requests_mock_datadir, cli_runner ):