diff --git a/requirements-swh.txt b/requirements-swh.txt --- a/requirements-swh.txt +++ b/requirements-swh.txt @@ -1,3 +1,3 @@ swh.core[http] >= 0.3 -swh.deposit +swh.deposit >= 0.3 swh.storage >= 0.0.162 diff --git a/swh/icinga_plugins/deposit.py b/swh/icinga_plugins/deposit.py --- a/swh/icinga_plugins/deposit.py +++ b/swh/icinga_plugins/deposit.py @@ -1,4 +1,4 @@ -# Copyright (C) 2019 The Software Heritage developers +# Copyright (C) 2019-2020 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -6,6 +6,7 @@ import datetime import sys import time +from typing import Any, Dict, Optional from swh.deposit.client import PublicApiDepositClient @@ -23,6 +24,7 @@ self._archive_path = obj["archive"] self._metadata_path = obj["metadata"] self._collection = obj["collection"] + self._slug: Optional[str] = None self._client = PublicApiDepositClient( { @@ -32,16 +34,36 @@ ) def upload_deposit(self): + slug = "check-deposit-%s" % datetime.datetime.now().isoformat() result = self._client.deposit_create( archive=self._archive_path, metadata=self._metadata_path, collection=self._collection, in_progress=False, - slug="check-deposit-%s" % datetime.datetime.now().isoformat(), + slug=slug, ) + self._slug = slug self._deposit_id = result["deposit_id"] return result + def update_deposit_with_metadata(self) -> Dict[str, Any]: + """Trigger a metadata update on the deposit once it's completed. + + """ + deposit = self.get_deposit_status() + assert deposit["deposit_status"] == "done" + swhid = deposit["deposit_swh_id"] + assert deposit["deposit_id"] == self._deposit_id + + # We can reuse the initial metadata file we already sent + return self._client.deposit_update( + self._collection, + self._deposit_id, + self._slug, + metadata=self._metadata_path, + swhid=swhid, + ) + def get_deposit_status(self): return self._client.deposit_status( collection=self._collection, deposit_id=self._deposit_id @@ -125,4 +147,35 @@ f'Deposit took {metrics["total_time"]:.2f}s and succeeded.', **metrics, ) + + if status_code != 0: # Stop if any problems + return status_code + + # Initial deposit scenario is done, now we can update the deposit with metadata + result = self.update_deposit_with_metadata() + total_time = time.time() - start_time + metrics_update = { + "total_time": total_time, + "update_time": ( + total_time + - metrics["upload_time"] + - metrics["validation_time"] + - metrics["load_time"] + ), + } + if "error" in result: + self.print_result( + "CRITICAL", + f'Deposit metadata update failed: {result["error"]} ', + **metrics_update, + ) + + (status_code, status) = self.get_status(metrics_update["total_time"]) + self.print_result( + status, + f'Deposit Metadata update took {metrics_update["update_time"]:.2f}s ' + "and succeeded.", + **metrics_update, + ) + return status_code diff --git a/swh/icinga_plugins/tests/test_deposit.py b/swh/icinga_plugins/tests/test_deposit.py --- a/swh/icinga_plugins/tests/test_deposit.py +++ b/swh/icinga_plugins/tests/test_deposit.py @@ -1,4 +1,4 @@ -# Copyright (C) 2019 The Software Heritage developers +# Copyright (C) 2019-2020 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -7,6 +7,7 @@ import os import tarfile import time +from typing import Optional from click.testing import CliRunner import pytest @@ -63,11 +64,69 @@ xmlns:dcterms="http://purl.org/dc/terms/"> 42 {status} - {status_detail} + {status_detail}%s """ +def status_template( + status: str, status_detail: str = "", swhid: Optional[str] = None +) -> str: + """Generate a proper status template out of status, status_detail and optional swhid + + """ + if swhid is not None: + template = STATUS_TEMPLATE % f"\n {swhid}" + return template.format(status=status, status_detail=status_detail, swhid=swhid) + template = STATUS_TEMPLATE % "" + return template.format(status=status, status_detail=status_detail) + + +def test_status_template(): + actual_status = status_template(status="deposited") + assert ( + actual_status + == """ + + 42 + deposited + + +""" + ) + + actual_status = status_template(status="verified", status_detail="detail") + assert ( + actual_status + == """ + + 42 + verified + detail + +""" + ) + + actual_status = status_template(status="done", swhid="10") + assert ( + actual_status + == """ + + 42 + done + + 10 + +""" + ) + + @pytest.fixture(scope="session") def tmp_path(tmp_path_factory): return tmp_path_factory.mktemp(__name__) @@ -113,8 +172,32 @@ ): scenario = WebScenario() + status_xml = status_template( + status="done", + status_detail="", + swhid="swh:1:dir:02ed6084fb0e8384ac58980e07548a547431cf74", + ) + + # Initial deposit + scenario.add_step( + "post", f"{BASE_URL}/testcol/", ENTRY_TEMPLATE.format(status="done") + ) + # Then metadata update + status_xml = status_template( + status="done", + status_detail="", + swhid="swh:1:dir:02ed6084fb0e8384ac58980e07548a547431cf74", + ) + scenario.add_step("get", f"{BASE_URL}/testcol/42/status/", status_xml) + # internal deposit client does call status, then update metadata then status api + scenario.add_step( + "get", f"{BASE_URL}/testcol/42/status/", status_xml, + ) + scenario.add_step( + "put", f"{BASE_URL}/testcol/42/metadata/", status_xml, + ) scenario.add_step( - "post", BASE_URL + "/testcol/", ENTRY_TEMPLATE.format(status="done") + "get", f"{BASE_URL}/testcol/42/status/", status_xml, ) scenario.install_mock(requests_mock) @@ -137,8 +220,11 @@ "| 'total_time' = 0.00s\n" "| 'upload_time' = 0.00s\n" "| 'validation_time' = 0.00s\n" + "DEPOSIT OK - Deposit Metadata update took 0.00s and succeeded.\n" + "| 'total_time' = 0.00s\n" + "| 'update_time' = 0.00s\n" ) - assert result.exit_code == 0, result.output + assert result.exit_code == 0, f"Unexpected output: {result.output}" def test_deposit_delays( @@ -150,19 +236,30 @@ "post", BASE_URL + "/testcol/", ENTRY_TEMPLATE.format(status="deposited") ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="verified", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="verified"), ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="loading", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="loading"), ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="done", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="done"), + ) + # Then metadata update + status_xml = status_template( + status="done", + status_detail="", + swhid="swh:1:dir:02ed6084fb0e8384ac58980e07548a547431cf74", + ) + scenario.add_step("get", f"{BASE_URL}/testcol/42/status/", status_xml) + # internal deposit client does call status, then update metadata then status api + scenario.add_step( + "get", f"{BASE_URL}/testcol/42/status/", status_xml, + ) + scenario.add_step( + "put", f"{BASE_URL}/testcol/42/metadata/", status_xml, + ) + scenario.add_step( + "get", f"{BASE_URL}/testcol/42/status/", status_xml, ) scenario.install_mock(requests_mock) @@ -185,6 +282,9 @@ "| 'total_time' = 30.00s\n" "| 'upload_time' = 0.00s\n" "| 'validation_time' = 10.00s\n" + "DEPOSIT OK - Deposit Metadata update took 0.00s and succeeded.\n" + "| 'total_time' = 30.00s\n" + "| 'update_time' = 0.00s\n" ) assert result.exit_code == 0, result.output @@ -198,14 +298,10 @@ "post", BASE_URL + "/testcol/", ENTRY_TEMPLATE.format(status="deposited") ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="verified", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="verified"), ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="done", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="done"), ) scenario.install_mock(requests_mock) @@ -244,14 +340,12 @@ "post", BASE_URL + "/testcol/", ENTRY_TEMPLATE.format(status="deposited") ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="verified", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="verified"), ) scenario.add_step( "get", BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="done", status_detail=""), + status_template(status="done"), callback=lambda: time.sleep(60), ) @@ -296,13 +390,13 @@ scenario.add_step( "get", BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="verified", status_detail=""), + status_template(status="verified"), callback=lambda: time.sleep(1500), ) scenario.add_step( "get", BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="loading", status_detail=""), + status_template(status="loading"), callback=lambda: time.sleep(1500), ) @@ -342,7 +436,7 @@ scenario.add_step( "get", BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="rejected", status_detail="booo"), + status_template(status="rejected", status_detail="booo"), ) scenario.install_mock(requests_mock) @@ -378,19 +472,15 @@ "post", BASE_URL + "/testcol/", ENTRY_TEMPLATE.format(status="deposited") ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="verified", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="verified"), ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="loading", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="loading"), ) scenario.add_step( "get", BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="failed", status_detail="booo"), + status_template(status="failed", status_detail="booo"), ) scenario.install_mock(requests_mock) @@ -427,19 +517,15 @@ "post", BASE_URL + "/testcol/", ENTRY_TEMPLATE.format(status="deposited") ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="verified", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="verified"), ) scenario.add_step( - "get", - BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="loading", status_detail=""), + "get", BASE_URL + "/testcol/42/status/", status_template(status="loading"), ) scenario.add_step( "get", BASE_URL + "/testcol/42/status/", - STATUS_TEMPLATE.format(status="what", status_detail="booo"), + status_template(status="what", status_detail="booo"), ) scenario.install_mock(requests_mock)