From baedf0405c5224798ac38dafdf37f9be6752b59a Mon Sep 17 00:00:00 2001 From: tcezard Date: Mon, 9 Feb 2026 16:54:26 +0000 Subject: [PATCH 01/12] Add Call home methods --- eva_sub_cli/call_home.py | 115 ++++++++++ eva_sub_cli/etc/call_home_payload_schema.json | 73 +++++++ eva_sub_cli/executables/cli.py | 34 ++- eva_sub_cli/orchestrator.py | 4 +- eva_sub_cli/submission_ws.py | 2 +- tests/test_call_home.py | 204 ++++++++++++++++++ tests/test_cli.py | 49 ++++- 7 files changed, 476 insertions(+), 5 deletions(-) create mode 100644 eva_sub_cli/call_home.py create mode 100644 eva_sub_cli/etc/call_home_payload_schema.json create mode 100644 tests/test_call_home.py diff --git a/eva_sub_cli/call_home.py b/eva_sub_cli/call_home.py new file mode 100644 index 0000000..db15227 --- /dev/null +++ b/eva_sub_cli/call_home.py @@ -0,0 +1,115 @@ +import logging +import os +import traceback +import uuid +from datetime import datetime, timezone + +import requests +from ebi_eva_common_pyutils.config import WritableConfig + +from eva_sub_cli import __version__, SUBMISSION_WS_VAR, SUB_CLI_CONFIG_FILE +from eva_sub_cli.submission_ws import DEFAULT_SUBMISSION_WS_URL + +logger = logging.getLogger(__name__) + +CALL_HOME_PATH = 'call-home' +DEPLOYMENT_ID_DIR = os.path.join(os.path.expanduser('~'), '.eva-sub-cli') +DEPLOYMENT_ID_FILE = os.path.join(DEPLOYMENT_ID_DIR, 'deployment_id') + +EVENT_START = 'START' +EVENT_VALIDATION_COMPLETED = 'VALIDATION_COMPLETED' +EVENT_END = 'END' +EVENT_FAILURE = 'FAILURE' + + +def _get_call_home_url(): + base_url = os.environ.get(SUBMISSION_WS_VAR) or DEFAULT_SUBMISSION_WS_URL + return os.path.join(base_url, CALL_HOME_PATH) + + +def _get_or_create_deployment_id(): + try: + if os.path.isfile(DEPLOYMENT_ID_FILE): + with open(DEPLOYMENT_ID_FILE, 'r') as f: + deployment_id = f.read().strip() + if deployment_id: + return deployment_id + deployment_id = str(uuid.uuid4()) + os.makedirs(DEPLOYMENT_ID_DIR, exist_ok=True) + with open(DEPLOYMENT_ID_FILE, 'w') as f: + f.write(deployment_id) + return deployment_id + except Exception: + logger.debug('Failed to read or create deployment ID file, using transient ID') + return str(uuid.uuid4()) + + +def _get_or_create_run_id(submission_dir): + try: + config_path = os.path.join(submission_dir, SUB_CLI_CONFIG_FILE) + config = WritableConfig(config_path) + run_id = config.get('run_id') + if run_id: + return str(run_id) + run_id = str(uuid.uuid4()) + config.set('run_id', value=run_id) + config.write() + return run_id + except Exception: + logger.debug('Failed to read or create run ID in config, using transient ID') + return str(uuid.uuid4()) + + +class CallHomeClient: + + def __init__(self, submission_dir, executor, tasks): + self.start_time = datetime.now(timezone.utc) + self.deployment_id = _get_or_create_deployment_id() + self.run_id = _get_or_create_run_id(submission_dir) + self.executor = executor + self.tasks = tasks + + def _build_payload(self, event_type, **kwargs): + if event_type == EVENT_START: + runtime_seconds = 0 + else: + elapsed = datetime.now(timezone.utc) - self.start_time + runtime_seconds = int(elapsed.total_seconds()) + payload = { + 'deploymentId': self.deployment_id, + 'runId': self.run_id, + 'eventType': event_type, + 'cliVersion': __version__, + 'createdAt': datetime.now(timezone.utc).isoformat(), + 'runtimeSeconds': runtime_seconds, + 'executor': self.executor, + 'tasks': self.tasks, + } + if kwargs: + payload.update(kwargs) + return payload + + def _send_event(self, event_type, **kwargs): + try: + payload = self._build_payload(event_type, **kwargs) + requests.post(_get_call_home_url(), json=payload, timeout=5) + except Exception: + logger.debug('Failed to send %s call-home event', event_type) + + def send_start(self): + self._send_event(EVENT_START) + + def send_validation_completed(self, validation_result): + self._send_event(EVENT_VALIDATION_COMPLETED, validation_result=validation_result) + + def send_end(self): + self._send_event(EVENT_END) + + def send_failure(self, exception=None): + kwargs = {} + if exception is not None: + kwargs['exceptionMessage'] = str(exception) + kwargs['exceptionStacktrace'] = ''.join( + traceback.format_exception(type(exception), exception, exception.__traceback__) + ) + self._send_event(EVENT_FAILURE, **kwargs) diff --git a/eva_sub_cli/etc/call_home_payload_schema.json b/eva_sub_cli/etc/call_home_payload_schema.json new file mode 100644 index 0000000..aff9bda --- /dev/null +++ b/eva_sub_cli/etc/call_home_payload_schema.json @@ -0,0 +1,73 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "Call-Home Event Payload", + "description": "Payload sent by the eva-sub-cli to the submission WS /call-home endpoint for usage telemetry.", + "type": "object", + "properties": { + "deploymentId": { + "type": "string", + "format": "uuid", + "description": "Persistent UUID identifying a unique CLI installation. Stored at ~/.eva-sub-cli/deployment_id." + }, + "runId": { + "type": "string", + "format": "uuid", + "description": "Persistent UUID identifying a submission directory run. Stored in the .eva_sub_cli_config.yml file within the submission directory." + }, + "eventType": { + "type": "string", + "enum": ["START", "VALIDATION_COMPLETED", "END", "FAILURE"], + "description": "Type of lifecycle event. START is sent when main() begins, VALIDATION_COMPLETED after successful validation, END on successful completion, FAILURE on any error." + }, + "cliVersion": { + "type": "string", + "description": "Version of the eva-sub-cli package (e.g. '0.5.3')." + }, + "createdAt": { + "type": "string", + "format": "date-time", + "description": "ISO 8601 UTC timestamp of when the event was created." + }, + "runtimeSeconds": { + "type": "integer", + "minimum": 0, + "description": "Elapsed wall-clock seconds since the CLI started. Always 0 for START events." + }, + "executor": { + "type": "string", + "enum": ["native", "docker"], + "description": "Execution backend selected for validation." + }, + "tasks": { + "type": "array", + "items": { + "type": "string", + "enum": ["validate", "submit"] + }, + "description": "List of tasks requested by the user." + }, + "validation_result": { + "type": "string", + "description": "Outcome of the validation step. Only present in VALIDATION_COMPLETED events." + }, + "exceptionMessage": { + "type": "string", + "description": "The exception message string. Only present in FAILURE events when an exception was caught." + }, + "exceptionStacktrace": { + "type": "string", + "description": "Full Python stacktrace of the exception. Only present in FAILURE events when an exception was caught." + } + }, + "required": [ + "deploymentId", + "runId", + "eventType", + "cliVersion", + "createdAt", + "runtimeSeconds", + "executor", + "tasks" + ], + "additionalProperties": true +} diff --git a/eva_sub_cli/executables/cli.py b/eva_sub_cli/executables/cli.py index 88e43ee..7a21752 100755 --- a/eva_sub_cli/executables/cli.py +++ b/eva_sub_cli/executables/cli.py @@ -19,6 +19,7 @@ MetadataTemplateVersionNotFoundException from eva_sub_cli.exceptions.submission_status_exception import SubmissionStatusException from eva_sub_cli.exceptions.submission_not_found_exception import SubmissionNotFoundException +from eva_sub_cli.call_home import CallHomeClient from eva_sub_cli.file_utils import is_submission_dir_writable, DirLockError, DirLock from eva_sub_cli.orchestrator import VALIDATE, SUBMIT, DOCKER, NATIVE from eva_sub_cli.validators.validator import ALL_VALIDATION_TASKS @@ -107,42 +108,71 @@ def main(): else: logging_config.add_stdout_handler(logging.INFO) + # Initialize call-home (fire-and-forget, guarded) + call_home = None + try: + call_home = CallHomeClient( + submission_dir=args.submission_dir, + executor=args.executor, + tasks=args.tasks + ) + call_home.send_start() + except Exception: + pass + + caught_exception = None try: # lock the submission directory with DirLock(os.path.join(args.submission_dir)) as lock: # Create the log file logging_config.add_file_handler(os.path.join(args.submission_dir, 'eva_submission.log'), logging.DEBUG) # Pass on all the arguments to the orchestrator - orchestrator.orchestrate_process(**args.__dict__) - except DirLockError: + orchestrator.orchestrate_process(call_home=call_home, **args.__dict__) + except DirLockError as dle: print(f'Could not acquire the lock file for {args.submission_dir} because another process is using this ' f'directory or a previous process did not terminate correctly. ' f'If the problem persists, remove the lock file manually.') + caught_exception = dle exit_status = 65 except FileNotFoundError as fne: print(fne) + caught_exception = fne exit_status = 66 except SubmissionNotFoundException as snfe: print(f'{snfe}. Please contact EVA Helpdesk') + caught_exception = snfe exit_status = 67 except SubmissionStatusException as sse: print(f'{sse}. Please try again later. If the problem persists, please contact EVA Helpdesk') + caught_exception = sse exit_status = 68 except MetadataTemplateVersionException as mte: print(mte) + caught_exception = mte exit_status = 69 except MetadataTemplateVersionNotFoundException as mte: print(mte) + caught_exception = mte exit_status = 70 except SubmissionUploadException as sue: print(sue) + caught_exception = sue exit_status = 71 except HTTPError as http_err: print(http_err) if http_err.response is not None and http_err.response.text: print(http_err.response.text) + caught_exception = http_err exit_status = 72 except Exception as ex: print(ex) + caught_exception = ex exit_status = 73 + + if call_home is not None: + if exit_status == 0: + call_home.send_end() + else: + call_home.send_failure(caught_exception) + return exit_status diff --git a/eva_sub_cli/orchestrator.py b/eva_sub_cli/orchestrator.py index e5f2526..52d8d0c 100755 --- a/eva_sub_cli/orchestrator.py +++ b/eva_sub_cli/orchestrator.py @@ -299,7 +299,7 @@ def check_validation_required(tasks, sub_config, username=None, password=None): def orchestrate_process(submission_dir, metadata_json, metadata_xlsx, tasks, executor, validation_tasks=ALL_VALIDATION_TASKS, username=None, password=None, - shallow_validation=False, nextflow_config=None, **kwargs): + shallow_validation=False, nextflow_config=None, call_home=None, **kwargs): # load config config_file_path = os.path.join(submission_dir, SUB_CLI_CONFIG_FILE) sub_config = WritableConfig(config_file_path, version=__version__) @@ -341,6 +341,8 @@ def orchestrate_process(submission_dir, metadata_json, metadata_xlsx, nextflow_config=nextflow_config) with validator: validator.validate_and_report() + if call_home: + call_home.send_validation_completed(validator.results) if not metadata_json: metadata_json = os.path.join(validator.output_dir, 'metadata.json') sub_config.set('metadata_json', value=metadata_json) diff --git a/eva_sub_cli/submission_ws.py b/eva_sub_cli/submission_ws.py index eb94a84..efb2086 100644 --- a/eva_sub_cli/submission_ws.py +++ b/eva_sub_cli/submission_ws.py @@ -9,6 +9,7 @@ from eva_sub_cli.auth import get_auth from eva_sub_cli.exceptions.submission_upload_exception import SubmissionUploadException +DEFAULT_SUBMISSION_WS_URL = 'https://www.ebi.ac.uk/eva/webservices/submission-ws/v1/' class SubmissionWSClient(AppLogger): """ @@ -19,7 +20,6 @@ def __init__(self, username=None, password=None): self.auth = get_auth(username, password) self.base_url = self._submission_ws_url - SUBMISSION_WS_URL = 'https://www.ebi.ac.uk/eva/webservices/submission-ws/v1/' SUBMISSION_INITIATE_PATH = 'submission/initiate' SUBMISSION_UPLOADED_PATH = 'submission/{submissionId}/uploaded' SUBMISSION_STATUS_PATH = 'submission/{submissionId}/status' diff --git a/tests/test_call_home.py b/tests/test_call_home.py new file mode 100644 index 0000000..6565541 --- /dev/null +++ b/tests/test_call_home.py @@ -0,0 +1,204 @@ +import os +import shutil +import tempfile +import uuid +from unittest import TestCase +from unittest.mock import patch, MagicMock + +from requests.exceptions import ConnectionError, Timeout + +from eva_sub_cli import SUB_CLI_CONFIG_FILE +from eva_sub_cli.call_home import ( + _get_or_create_deployment_id, _get_or_create_run_id, + CallHomeClient, EVENT_START, EVENT_VALIDATION_COMPLETED, EVENT_END, EVENT_FAILURE, + DEPLOYMENT_ID_FILE, DEPLOYMENT_ID_DIR, _get_call_home_url +) + + +class TestDeploymentId(TestCase): + + resources_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'resources') + + def setUp(self): + self.config_dir = os.path.join(self.resources_dir, '.eva-sub-cli') + self.deployment_file = os.path.join(self.config_dir, 'deployment_id') + + def tearDown(self): + shutil.rmtree(self.config_dir) + + def test_creates_new_id_when_file_missing(self): + with patch('eva_sub_cli.call_home.DEPLOYMENT_ID_DIR', self.config_dir), \ + patch('eva_sub_cli.call_home.DEPLOYMENT_ID_FILE', self.deployment_file): + deployment_id = _get_or_create_deployment_id() + # Should be a valid UUID + uuid.UUID(deployment_id) + # File should have been created + self.assertTrue(os.path.isfile(self.deployment_file)) + with open(self.deployment_file) as f: + self.assertEqual(f.read().strip(), deployment_id) + + def test_reads_existing_id(self): + os.makedirs(self.config_dir, exist_ok=True) + existing_id = str(uuid.uuid4()) + with open(self.deployment_file, 'w') as f: + f.write(existing_id) + + with patch('eva_sub_cli.call_home.DEPLOYMENT_ID_DIR', self.config_dir), \ + patch('eva_sub_cli.call_home.DEPLOYMENT_ID_FILE', self.deployment_file): + deployment_id = _get_or_create_deployment_id() + self.assertEqual(deployment_id, existing_id) + + def test_handles_permission_error(self): + with patch('eva_sub_cli.call_home.DEPLOYMENT_ID_DIR', '/nonexistent/path'), \ + patch('eva_sub_cli.call_home.DEPLOYMENT_ID_FILE', '/nonexistent/path/deployment_id'): + deployment_id = _get_or_create_deployment_id() + # Should return a transient UUID without raising + uuid.UUID(deployment_id) + + def test_regenerates_on_empty_file(self): + os.makedirs(self.config_dir, exist_ok=True) + with open(self.deployment_file, 'w') as f: + f.write('') + + with patch('eva_sub_cli.call_home.DEPLOYMENT_ID_DIR', self.mock_dir), \ + patch('eva_sub_cli.call_home.DEPLOYMENT_ID_FILE', self.deployment_file): + deployment_id = _get_or_create_deployment_id() + uuid.UUID(deployment_id) + with open(self.deployment_file) as f: + self.assertEqual(f.read().strip(), deployment_id) + + +class TestRunId(TestCase): + + resources_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'resources') + + def setUp(self): + self.submission_dir = os.path.join(self.resources_dir, 'submission_dir') + + def tearDown(self): + shutil.rmtree(self.submission_dir) + + def test_creates_new_id_when_config_has_no_run_id(self): + run_id = _get_or_create_run_id(self.submission_dir) + uuid.UUID(run_id) + # Verify it was written to config + config_path = os.path.join(self.submission_dir, SUB_CLI_CONFIG_FILE) + self.assertTrue(os.path.isfile(config_path)) + + def test_reads_existing_id_from_config(self): + existing_id = str(uuid.uuid4()) + # Create config with an existing run_id + from ebi_eva_common_pyutils.config import WritableConfig + config_path = os.path.join(self.submission_dir, SUB_CLI_CONFIG_FILE) + config = WritableConfig(config_path) + config.set('run_id', value=existing_id) + config.write() + + run_id = _get_or_create_run_id(self.submission_dir) + self.assertEqual(run_id, existing_id) + + def test_handles_unreadable_config(self): + with patch('eva_sub_cli.call_home.WritableConfig', side_effect=Exception('Cannot read config')): + run_id = _get_or_create_run_id(self.submission_dir) + uuid.UUID(run_id) + + +class TestCallHomeClient(TestCase): + + def setUp(self): + self.tmp_dir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.tmp_dir) + + @patch('eva_sub_cli.call_home._get_or_create_run_id', return_value='test-run-id') + @patch('eva_sub_cli.call_home._get_or_create_deployment_id', return_value='test-deployment-id') + def _create_client(self, mock_dep_id, mock_run_id): + return CallHomeClient( + submission_dir=self.tmp_dir, + executor='native', + tasks=['validate', 'submit'] + ) + + def test_start_payload_has_zero_runtime(self): + client = self._create_client() + payload = client._build_payload(EVENT_START) + self.assertEqual(payload['runtimeSeconds'], 0) + self.assertEqual(payload['eventType'], EVENT_START) + self.assertEqual(payload['deploymentId'], 'test-deployment-id') + self.assertEqual(payload['runId'], 'test-run-id') + self.assertEqual(payload['executor'], 'native') + self.assertEqual(payload['tasks'], ['validate', 'submit']) + self.assertIn('cliVersion', payload) + self.assertIn('createdAt', payload) + + def test_validation_completed_payload(self): + client = self._create_client() + payload = client._build_payload(EVENT_VALIDATION_COMPLETED) + self.assertEqual(payload['eventType'], EVENT_VALIDATION_COMPLETED) + self.assertIsInstance(payload['runtimeSeconds'], int) + + def test_end_payload(self): + client = self._create_client() + payload = client._build_payload(EVENT_END) + self.assertEqual(payload['eventType'], EVENT_END) + + def test_failure_payload(self): + client = self._create_client() + payload = client._build_payload(EVENT_FAILURE) + self.assertEqual(payload['eventType'], EVENT_FAILURE) + + @patch('eva_sub_cli.call_home.requests.post') + def test_send_event_posts_with_correct_args(self, mock_post): + client = self._create_client() + client.send_start() + mock_post.assert_called_once() + call_kwargs = mock_post.call_args + self.assertEqual(call_kwargs.kwargs['timeout'], 5) + self.assertIn('json', call_kwargs.kwargs) + + @patch('eva_sub_cli.call_home.requests.post', side_effect=ConnectionError('connection refused')) + def test_connection_error_swallowed(self, mock_post): + client = self._create_client() + # Should not raise + client.send_start() + + @patch('eva_sub_cli.call_home.requests.post', side_effect=Timeout('timed out')) + def test_timeout_swallowed(self, mock_post): + client = self._create_client() + client.send_end() + + @patch('eva_sub_cli.call_home.requests.post', side_effect=Exception('unexpected error')) + def test_generic_exception_swallowed(self, mock_post): + client = self._create_client() + client.send_failure() + + @patch('eva_sub_cli.call_home.requests.post') + def test_send_all_event_types(self, mock_post): + client = self._create_client() + client.send_start() + client.send_validation_completed('PASS') + client.send_end() + self.assertEqual(mock_post.call_count, 3) + + @patch('eva_sub_cli.call_home.requests.post') + def test_send_failure_event(self, mock_post): + client = self._create_client() + client.send_failure() + payload = mock_post.call_args.kwargs['json'] + self.assertEqual(payload['eventType'], EVENT_FAILURE) + self.assertNotIn('exceptionMessage', payload) + self.assertNotIn('exceptionStacktrace', payload) + + @patch('eva_sub_cli.call_home.requests.post') + def test_send_failure_with_exception(self, mock_post): + client = self._create_client() + try: + raise ValueError('something went wrong') + except ValueError as e: + client.send_failure(exception=e) + payload = mock_post.call_args.kwargs['json'] + self.assertEqual(payload['eventType'], EVENT_FAILURE) + self.assertEqual(payload['exceptionMessage'], 'something went wrong') + self.assertIn('ValueError', payload['exceptionStacktrace']) + self.assertIn('something went wrong', payload['exceptionStacktrace']) diff --git a/tests/test_cli.py b/tests/test_cli.py index 674a7db..3f87c24 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -36,7 +36,8 @@ def test_main(self): vcf_files=[], reference_fasta='', metadata_json=None, metadata_xlsx='', tasks='validate', executor='native', debug=False) with patch('eva_sub_cli.executables.cli.parse_args', return_value=args), \ - patch('eva_sub_cli.orchestrator.orchestrate_process'): + patch('eva_sub_cli.orchestrator.orchestrate_process'), \ + patch('eva_sub_cli.executables.cli.CallHomeClient'): exit_status = cli.main() # Check that the debug message is shown logger = orchestrator.logger @@ -94,6 +95,7 @@ def test_main_exception_handling(self): with patch('eva_sub_cli.executables.cli.parse_args', return_value=args), \ patch('eva_sub_cli.executables.cli.orchestrator.orchestrate_process', side_effect=exception), \ + patch('eva_sub_cli.executables.cli.CallHomeClient'), \ patch('builtins.print') as mock_print: exit_status = cli.main() @@ -107,3 +109,48 @@ def test_main_exception_handling(self): if (isinstance(exception, HTTPError)): self.assertIn(exception.response.text, printed_texts) + + def test_main_sends_start_and_end_on_success(self): + args = Mock(submission_dir=self.submission_dir, + vcf_files=[], reference_fasta='', metadata_json=None, metadata_xlsx='', + tasks=['validate'], executor='native', debug=False) + with patch('eva_sub_cli.executables.cli.parse_args', return_value=args), \ + patch('eva_sub_cli.orchestrator.orchestrate_process'), \ + patch('eva_sub_cli.executables.cli.CallHomeClient') as MockCallHome: + mock_client = MockCallHome.return_value + exit_status = cli.main() + self.assertEqual(exit_status, 0) + mock_client.send_start.assert_called_once() + mock_client.send_end.assert_called_once() + mock_client.send_failure.assert_not_called() + + def test_main_sends_start_and_failure_on_exception(self): + args = Mock(submission_dir=self.submission_dir, + vcf_files=[], reference_fasta='', metadata_json=None, metadata_xlsx='', + tasks=['submit'], executor='native', debug=False) + exception = Exception('boom') + with patch('eva_sub_cli.executables.cli.parse_args', return_value=args), \ + patch('eva_sub_cli.executables.cli.orchestrator.orchestrate_process', + side_effect=exception), \ + patch('eva_sub_cli.executables.cli.CallHomeClient') as MockCallHome, \ + patch('builtins.print'): + mock_client = MockCallHome.return_value + exit_status = cli.main() + self.assertNotEqual(exit_status, 0) + mock_client.send_start.assert_called_once() + mock_client.send_failure.assert_called_once() + failure_arg = mock_client.send_failure.call_args[0][0] + self.assertIsInstance(failure_arg, Exception) + self.assertEqual(str(failure_arg), 'boom') + mock_client.send_end.assert_not_called() + + def test_main_unaffected_by_call_home_init_failure(self): + args = Mock(submission_dir=self.submission_dir, + vcf_files=[], reference_fasta='', metadata_json=None, metadata_xlsx='', + tasks=['validate'], executor='native', debug=False) + with patch('eva_sub_cli.executables.cli.parse_args', return_value=args), \ + patch('eva_sub_cli.orchestrator.orchestrate_process'), \ + patch('eva_sub_cli.executables.cli.CallHomeClient', + side_effect=Exception('call home init failed')): + exit_status = cli.main() + self.assertEqual(exit_status, 0) From 1d2335e55f900e7912c2e18d457ed5bff59bfbe5 Mon Sep 17 00:00:00 2001 From: tcezard Date: Tue, 10 Feb 2026 15:53:54 +0000 Subject: [PATCH 02/12] refactor and simplify --- eva_sub_cli/call_home.py | 9 +++++---- eva_sub_cli/executables/cli.py | 2 +- eva_sub_cli/submission_ws.py | 16 ++++++++++------ tests/test_call_home.py | 18 ++++++++++++------ 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/eva_sub_cli/call_home.py b/eva_sub_cli/call_home.py index db15227..2a2b7bd 100644 --- a/eva_sub_cli/call_home.py +++ b/eva_sub_cli/call_home.py @@ -7,8 +7,8 @@ import requests from ebi_eva_common_pyutils.config import WritableConfig -from eva_sub_cli import __version__, SUBMISSION_WS_VAR, SUB_CLI_CONFIG_FILE -from eva_sub_cli.submission_ws import DEFAULT_SUBMISSION_WS_URL +from eva_sub_cli import __version__, SUB_CLI_CONFIG_FILE +from eva_sub_cli.submission_ws import _submission_ws_base_url logger = logging.getLogger(__name__) @@ -23,8 +23,7 @@ def _get_call_home_url(): - base_url = os.environ.get(SUBMISSION_WS_VAR) or DEFAULT_SUBMISSION_WS_URL - return os.path.join(base_url, CALL_HOME_PATH) + return os.path.join(_submission_ws_base_url(), CALL_HOME_PATH) def _get_or_create_deployment_id(): @@ -65,6 +64,8 @@ class CallHomeClient: def __init__(self, submission_dir, executor, tasks): self.start_time = datetime.now(timezone.utc) self.deployment_id = _get_or_create_deployment_id() + if not os.path.exists(submission_dir): + os.makedirs(submission_dir) self.run_id = _get_or_create_run_id(submission_dir) self.executor = executor self.tasks = tasks diff --git a/eva_sub_cli/executables/cli.py b/eva_sub_cli/executables/cli.py index 7a21752..6a2c8b5 100755 --- a/eva_sub_cli/executables/cli.py +++ b/eva_sub_cli/executables/cli.py @@ -108,7 +108,7 @@ def main(): else: logging_config.add_stdout_handler(logging.INFO) - # Initialize call-home (fire-and-forget, guarded) + # Initialize call-home call_home = None try: call_home = CallHomeClient( diff --git a/eva_sub_cli/submission_ws.py b/eva_sub_cli/submission_ws.py index efb2086..4243e23 100644 --- a/eva_sub_cli/submission_ws.py +++ b/eva_sub_cli/submission_ws.py @@ -11,6 +11,15 @@ DEFAULT_SUBMISSION_WS_URL = 'https://www.ebi.ac.uk/eva/webservices/submission-ws/v1/' + +def _submission_ws_base_url(): + """Retrieve the base URL for the submission web services. + In order of preference from the environment variable or the hardcoded value.""" + if os.environ.get(SUBMISSION_WS_VAR): + return os.environ.get(SUBMISSION_WS_VAR) + else: + return DEFAULT_SUBMISSION_WS_URL + class SubmissionWSClient(AppLogger): """ Python client for interfacing with the Submission WS API. @@ -26,12 +35,7 @@ def __init__(self, username=None, password=None): @property def _submission_ws_url(self): - """Retrieve the base URL for the submission web services. - In order of preference from the environment variable or the hardcoded value.""" - if os.environ.get(SUBMISSION_WS_VAR): - return os.environ.get(SUBMISSION_WS_VAR) - else: - return self.SUBMISSION_WS_URL + return _submission_ws_base_url() def _submission_initiate_url(self): return os.path.join(self.base_url, self.SUBMISSION_INITIATE_PATH) diff --git a/tests/test_call_home.py b/tests/test_call_home.py index 6565541..16a3e84 100644 --- a/tests/test_call_home.py +++ b/tests/test_call_home.py @@ -24,7 +24,8 @@ def setUp(self): self.deployment_file = os.path.join(self.config_dir, 'deployment_id') def tearDown(self): - shutil.rmtree(self.config_dir) + if os.path.exists(self.config_dir):( + shutil.rmtree(self.config_dir)) def test_creates_new_id_when_file_missing(self): with patch('eva_sub_cli.call_home.DEPLOYMENT_ID_DIR', self.config_dir), \ @@ -60,7 +61,7 @@ def test_regenerates_on_empty_file(self): with open(self.deployment_file, 'w') as f: f.write('') - with patch('eva_sub_cli.call_home.DEPLOYMENT_ID_DIR', self.mock_dir), \ + with patch('eva_sub_cli.call_home.DEPLOYMENT_ID_DIR', self.config_dir), \ patch('eva_sub_cli.call_home.DEPLOYMENT_ID_FILE', self.deployment_file): deployment_id = _get_or_create_deployment_id() uuid.UUID(deployment_id) @@ -74,9 +75,11 @@ class TestRunId(TestCase): def setUp(self): self.submission_dir = os.path.join(self.resources_dir, 'submission_dir') + os.makedirs(self.submission_dir, exist_ok=True) def tearDown(self): - shutil.rmtree(self.submission_dir) + if os.path.exists(self.submission_dir): + shutil.rmtree(self.submission_dir) def test_creates_new_id_when_config_has_no_run_id(self): run_id = _get_or_create_run_id(self.submission_dir) @@ -105,17 +108,20 @@ def test_handles_unreadable_config(self): class TestCallHomeClient(TestCase): + resources_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'resources') + def setUp(self): - self.tmp_dir = tempfile.mkdtemp() + self.submission_dir = os.path.join(self.resources_dir, 'submission_dir') def tearDown(self): - shutil.rmtree(self.tmp_dir) + if os.path.exists(self.submission_dir): + shutil.rmtree(self.submission_dir) @patch('eva_sub_cli.call_home._get_or_create_run_id', return_value='test-run-id') @patch('eva_sub_cli.call_home._get_or_create_deployment_id', return_value='test-deployment-id') def _create_client(self, mock_dep_id, mock_run_id): return CallHomeClient( - submission_dir=self.tmp_dir, + submission_dir=self.submission_dir, executor='native', tasks=['validate', 'submit'] ) From 44454980f5331fc09644fe752bd8399ffde58ce4 Mon Sep 17 00:00:00 2001 From: tcezard Date: Tue, 10 Feb 2026 16:02:34 +0000 Subject: [PATCH 03/12] fix test --- tests/test_submit.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_submit.py b/tests/test_submit.py index 46c20d9..808b9a4 100644 --- a/tests/test_submit.py +++ b/tests/test_submit.py @@ -9,7 +9,7 @@ from eva_sub_cli import SUB_CLI_CONFIG_FILE from eva_sub_cli.file_utils import is_submission_dir_writable -from eva_sub_cli.submission_ws import SubmissionWSClient +from eva_sub_cli.submission_ws import SubmissionWSClient, DEFAULT_SUBMISSION_WS_URL from eva_sub_cli.validators.validator import READY_FOR_SUBMISSION_TO_EVA from eva_sub_cli.submit import StudySubmitter, SUB_CLI_CONFIG_KEY_SUBMISSION_ID, \ SUB_CLI_CONFIG_KEY_SUBMISSION_UPLOAD_URL @@ -58,11 +58,11 @@ def test_submit(self): self.submitter.submit() mock_post.assert_called_once_with( - os.path.join(test_submission_ws_client.SUBMISSION_WS_URL, 'submission/initiate'), + os.path.join(DEFAULT_SUBMISSION_WS_URL, 'submission/initiate'), headers={'Accept': 'application/json', 'Authorization': 'Bearer a token'}) mock_put.assert_called_once_with( - os.path.join(test_submission_ws_client.SUBMISSION_WS_URL, 'submission/mock_submission_id/uploaded'), + os.path.join(DEFAULT_SUBMISSION_WS_URL, 'submission/mock_submission_id/uploaded'), headers={'Accept': 'application/json', 'Authorization': 'Bearer a token'}, json=self.metadata_json) From 1fc465e91e18a5c8fa847a65acf8d2b2549f44c4 Mon Sep 17 00:00:00 2001 From: tcezard Date: Tue, 10 Feb 2026 16:08:47 +0000 Subject: [PATCH 04/12] test multiple python versions --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5858036..d6ed80e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.9] + python-version: [3.9, 3.10, 3.11, 3.12, 3.13, 3.14] steps: - uses: actions/checkout@v2 From 7cbcaffd0269d06b6f36ddaaba666fa4e67dadf6 Mon Sep 17 00:00:00 2001 From: tcezard Date: Tue, 10 Feb 2026 16:39:50 +0000 Subject: [PATCH 05/12] remove 3.10 --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d6ed80e..48ab2d5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.9, 3.10, 3.11, 3.12, 3.13, 3.14] + python-version: [3.9, 3.11, 3.12, 3.13, 3.14] steps: - uses: actions/checkout@v2 From 55d764cc61828fd16210e0cef4e63058c35b686e Mon Sep 17 00:00:00 2001 From: tcezard Date: Tue, 10 Feb 2026 18:51:16 +0000 Subject: [PATCH 06/12] Leave working python version --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 48ab2d5..e1c382f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.9, 3.11, 3.12, 3.13, 3.14] + python-version: ["3.9", "3.10", "3.11"] steps: - uses: actions/checkout@v2 From 6f4ca4ec3e62357714ed9779c177daa50ec5d10b Mon Sep 17 00:00:00 2001 From: tcezard Date: Tue, 10 Feb 2026 20:28:22 +0000 Subject: [PATCH 07/12] remove unused imports --- tests/test_call_home.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/test_call_home.py b/tests/test_call_home.py index 16a3e84..b5b9895 100644 --- a/tests/test_call_home.py +++ b/tests/test_call_home.py @@ -1,18 +1,14 @@ import os import shutil -import tempfile import uuid from unittest import TestCase -from unittest.mock import patch, MagicMock +from unittest.mock import patch from requests.exceptions import ConnectionError, Timeout from eva_sub_cli import SUB_CLI_CONFIG_FILE -from eva_sub_cli.call_home import ( - _get_or_create_deployment_id, _get_or_create_run_id, - CallHomeClient, EVENT_START, EVENT_VALIDATION_COMPLETED, EVENT_END, EVENT_FAILURE, - DEPLOYMENT_ID_FILE, DEPLOYMENT_ID_DIR, _get_call_home_url -) +from eva_sub_cli.call_home import _get_or_create_deployment_id, _get_or_create_run_id, \ + CallHomeClient, EVENT_START, EVENT_VALIDATION_COMPLETED, EVENT_END, EVENT_FAILURE class TestDeploymentId(TestCase): From 597fb2c86f4a96d0d3da22d9fb68668cd5724ec8 Mon Sep 17 00:00:00 2001 From: tcezard Date: Wed, 11 Feb 2026 10:19:21 +0000 Subject: [PATCH 08/12] Add python 3.12.3 --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e1c382f..ba76945 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.9", "3.10", "3.11"] + python-version: ["3.9", "3.10", "3.11", "3.12.3"] steps: - uses: actions/checkout@v2 From 1eff078b4956574bb821af888fa9b5ad94062310 Mon Sep 17 00:00:00 2001 From: tcezard Date: Wed, 11 Feb 2026 10:45:26 +0000 Subject: [PATCH 09/12] Fix test failing due to changes in v3.12 --- .github/workflows/tests.yml | 2 +- tests/test_cli.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ba76945..7078ed1 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.9", "3.10", "3.11", "3.12.3"] + python-version: ["3.9", "3.11", "3.12"] steps: - uses: actions/checkout@v2 diff --git a/tests/test_cli.py b/tests/test_cli.py index 3f87c24..ee9322d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -62,8 +62,9 @@ def test_validate_args(self): args = cli.parse_args(cmd_args) assert args.submission_dir == self.submission_dir - with patch('sys.exit') as m_exit: - cli.parse_args(cmd_args[:2]+cmd_args[4:]) + with patch('sys.exit', side_effect=SystemExit) as m_exit: + with self.assertRaises(SystemExit): + cli.parse_args(cmd_args[:2]+cmd_args[4:]) m_exit.assert_called_once_with(2) def test_main_exception_handling(self): From bb995395658f62f7c17238741e83b4cd3d338c2e Mon Sep 17 00:00:00 2001 From: tcezard Date: Thu, 12 Feb 2026 11:34:53 +0000 Subject: [PATCH 10/12] Address review comments --- eva_sub_cli/call_home.py | 7 ++-- eva_sub_cli/etc/call_home_payload_schema.json | 2 +- tests/test_call_home.py | 34 ++++--------------- 3 files changed, 10 insertions(+), 33 deletions(-) diff --git a/eva_sub_cli/call_home.py b/eva_sub_cli/call_home.py index 2a2b7bd..aab046c 100644 --- a/eva_sub_cli/call_home.py +++ b/eva_sub_cli/call_home.py @@ -64,24 +64,23 @@ class CallHomeClient: def __init__(self, submission_dir, executor, tasks): self.start_time = datetime.now(timezone.utc) self.deployment_id = _get_or_create_deployment_id() - if not os.path.exists(submission_dir): - os.makedirs(submission_dir) self.run_id = _get_or_create_run_id(submission_dir) self.executor = executor self.tasks = tasks def _build_payload(self, event_type, **kwargs): + now = datetime.now(timezone.utc) if event_type == EVENT_START: runtime_seconds = 0 else: - elapsed = datetime.now(timezone.utc) - self.start_time + elapsed = now - self.start_time runtime_seconds = int(elapsed.total_seconds()) payload = { 'deploymentId': self.deployment_id, 'runId': self.run_id, 'eventType': event_type, 'cliVersion': __version__, - 'createdAt': datetime.now(timezone.utc).isoformat(), + 'createdAt': now.isoformat(), 'runtimeSeconds': runtime_seconds, 'executor': self.executor, 'tasks': self.tasks, diff --git a/eva_sub_cli/etc/call_home_payload_schema.json b/eva_sub_cli/etc/call_home_payload_schema.json index aff9bda..f5a949f 100644 --- a/eva_sub_cli/etc/call_home_payload_schema.json +++ b/eva_sub_cli/etc/call_home_payload_schema.json @@ -17,7 +17,7 @@ "eventType": { "type": "string", "enum": ["START", "VALIDATION_COMPLETED", "END", "FAILURE"], - "description": "Type of lifecycle event. START is sent when main() begins, VALIDATION_COMPLETED after successful validation, END on successful completion, FAILURE on any error." + "description": "Type of lifecycle event. START is sent when main() begins, VALIDATION_COMPLETED after validation finishes, END when the program complete, FAILURE on any error." }, "cliVersion": { "type": "string", diff --git a/tests/test_call_home.py b/tests/test_call_home.py index b5b9895..d2b83d4 100644 --- a/tests/test_call_home.py +++ b/tests/test_call_home.py @@ -4,11 +4,12 @@ from unittest import TestCase from unittest.mock import patch +from ebi_eva_common_pyutils.config import Configuration, WritableConfig from requests.exceptions import ConnectionError, Timeout from eva_sub_cli import SUB_CLI_CONFIG_FILE from eva_sub_cli.call_home import _get_or_create_deployment_id, _get_or_create_run_id, \ - CallHomeClient, EVENT_START, EVENT_VALIDATION_COMPLETED, EVENT_END, EVENT_FAILURE + CallHomeClient, EVENT_START, EVENT_FAILURE class TestDeploymentId(TestCase): @@ -83,11 +84,12 @@ def test_creates_new_id_when_config_has_no_run_id(self): # Verify it was written to config config_path = os.path.join(self.submission_dir, SUB_CLI_CONFIG_FILE) self.assertTrue(os.path.isfile(config_path)) + config = Configuration(config_path) + self.assertEqual(config.query('run_id'), run_id) def test_reads_existing_id_from_config(self): existing_id = str(uuid.uuid4()) # Create config with an existing run_id - from ebi_eva_common_pyutils.config import WritableConfig config_path = os.path.join(self.submission_dir, SUB_CLI_CONFIG_FILE) config = WritableConfig(config_path) config.set('run_id', value=existing_id) @@ -108,6 +110,7 @@ class TestCallHomeClient(TestCase): def setUp(self): self.submission_dir = os.path.join(self.resources_dir, 'submission_dir') + os.makedirs(self.submission_dir, exist_ok=True) def tearDown(self): if os.path.exists(self.submission_dir): @@ -122,7 +125,7 @@ def _create_client(self, mock_dep_id, mock_run_id): tasks=['validate', 'submit'] ) - def test_start_payload_has_zero_runtime(self): + def test_build_payload(self): client = self._create_client() payload = client._build_payload(EVENT_START) self.assertEqual(payload['runtimeSeconds'], 0) @@ -134,31 +137,6 @@ def test_start_payload_has_zero_runtime(self): self.assertIn('cliVersion', payload) self.assertIn('createdAt', payload) - def test_validation_completed_payload(self): - client = self._create_client() - payload = client._build_payload(EVENT_VALIDATION_COMPLETED) - self.assertEqual(payload['eventType'], EVENT_VALIDATION_COMPLETED) - self.assertIsInstance(payload['runtimeSeconds'], int) - - def test_end_payload(self): - client = self._create_client() - payload = client._build_payload(EVENT_END) - self.assertEqual(payload['eventType'], EVENT_END) - - def test_failure_payload(self): - client = self._create_client() - payload = client._build_payload(EVENT_FAILURE) - self.assertEqual(payload['eventType'], EVENT_FAILURE) - - @patch('eva_sub_cli.call_home.requests.post') - def test_send_event_posts_with_correct_args(self, mock_post): - client = self._create_client() - client.send_start() - mock_post.assert_called_once() - call_kwargs = mock_post.call_args - self.assertEqual(call_kwargs.kwargs['timeout'], 5) - self.assertIn('json', call_kwargs.kwargs) - @patch('eva_sub_cli.call_home.requests.post', side_effect=ConnectionError('connection refused')) def test_connection_error_swallowed(self, mock_post): client = self._create_client() From c542492ce161f1ea0bb20dcb1cceb25a4b466115 Mon Sep 17 00:00:00 2001 From: tcezard Date: Thu, 12 Feb 2026 11:42:56 +0000 Subject: [PATCH 11/12] Add non sero runtime test --- tests/test_call_home.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_call_home.py b/tests/test_call_home.py index d2b83d4..10e09aa 100644 --- a/tests/test_call_home.py +++ b/tests/test_call_home.py @@ -1,5 +1,6 @@ import os import shutil +import time import uuid from unittest import TestCase from unittest.mock import patch @@ -9,7 +10,7 @@ from eva_sub_cli import SUB_CLI_CONFIG_FILE from eva_sub_cli.call_home import _get_or_create_deployment_id, _get_or_create_run_id, \ - CallHomeClient, EVENT_START, EVENT_FAILURE + CallHomeClient, EVENT_START, EVENT_FAILURE, EVENT_END class TestDeploymentId(TestCase): @@ -137,6 +138,13 @@ def test_build_payload(self): self.assertIn('cliVersion', payload) self.assertIn('createdAt', payload) + def test_non_zero_runtime_on_non_start(self): + client = self._create_client() + time.sleep(1) + payload = client._build_payload(EVENT_END) + self.assertNotEqual(payload['runtimeSeconds'], 0) + self.assertEqual(payload['eventType'], EVENT_END) + @patch('eva_sub_cli.call_home.requests.post', side_effect=ConnectionError('connection refused')) def test_connection_error_swallowed(self, mock_post): client = self._create_client() From 8c9da6cc61bfb5131e40f73c26346795f4d87b02 Mon Sep 17 00:00:00 2001 From: Timothee Cezard Date: Thu, 12 Feb 2026 14:12:43 +0000 Subject: [PATCH 12/12] Update eva_sub_cli/etc/call_home_payload_schema.json Co-authored-by: April Shen --- eva_sub_cli/etc/call_home_payload_schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eva_sub_cli/etc/call_home_payload_schema.json b/eva_sub_cli/etc/call_home_payload_schema.json index f5a949f..251a673 100644 --- a/eva_sub_cli/etc/call_home_payload_schema.json +++ b/eva_sub_cli/etc/call_home_payload_schema.json @@ -17,7 +17,7 @@ "eventType": { "type": "string", "enum": ["START", "VALIDATION_COMPLETED", "END", "FAILURE"], - "description": "Type of lifecycle event. START is sent when main() begins, VALIDATION_COMPLETED after validation finishes, END when the program complete, FAILURE on any error." + "description": "Type of lifecycle event. START is sent when main() begins, VALIDATION_COMPLETED after validation finishes, END when the program completes, FAILURE on any error." }, "cliVersion": { "type": "string",