diff --git a/azure-devops/azext_devops/dev/common/artifacttool.py b/azure-devops/azext_devops/dev/common/artifacttool.py index edaec4af..d5084f57 100644 --- a/azure-devops/azext_devops/dev/common/artifacttool.py +++ b/azure-devops/azext_devops/dev/common/artifacttool.py @@ -24,9 +24,11 @@ def download_pipeline_artifact(self, organization, project, run_id, artifact_nam "--project", project, "--pipeline-id", run_id, "--artifact-name", artifact_name, "--path", path] return self.run_artifacttool(organization, args, "Downloading") - def upload_pipeline_artifact(self, organization, project, run_id, artifact_name, path): + def upload_pipeline_artifact(self, organization, project, run_id, artifact_name, path, properties=None): args = ["pipelineartifact", "publish", "--service", organization, "--patvar", ARTIFACTTOOL_PAT_ENVKEY, "--project", project, "--pipeline-id", run_id, "--artifact-name", artifact_name, "--path", path] + if properties: + args.extend(["--properties", properties]) return self.run_artifacttool(organization, args, "Uploading") def download_universal(self, organization, project, feed, package_name, package_version, path, file_filter): diff --git a/azure-devops/azext_devops/dev/pipelines/_help.py b/azure-devops/azext_devops/dev/pipelines/_help.py index 1705c522..82b23c2e 100644 --- a/azure-devops/azext_devops/dev/pipelines/_help.py +++ b/azure-devops/azext_devops/dev/pipelines/_help.py @@ -109,6 +109,17 @@ def load_pipelines_help(): long-summary: """ + helps['pipelines runs artifact upload'] = """ + type: command + short-summary: Upload a pipeline artifact. + long-summary: > + Upload a pipeline artifact to a specific run. Optionally specify custom properties as metadata. + examples: + - name: Upload a pipeline artifact with custom properties + text: | + az pipelines runs artifact upload --artifact-name myArtifact --run-id 123 --path /path/to/artifact --properties "user-key1=value1;user-key2=value2" +""" + helps['pipelines create'] = """ type: command short-summary: Create a new Azure Pipeline (YAML based). diff --git a/azure-devops/azext_devops/dev/pipelines/arguments.py b/azure-devops/azext_devops/dev/pipelines/arguments.py index 3d675999..04b1dabb 100644 --- a/azure-devops/azext_devops/dev/pipelines/arguments.py +++ b/azure-devops/azext_devops/dev/pipelines/arguments.py @@ -119,3 +119,10 @@ def load_build_arguments(self, _): with self.argument_context('pipelines folder') as context: context.argument('query_order', **enum_choice_list(_FOLDERS_QUERY_ORDER)) + + with self.argument_context('pipelines runs artifact upload') as context: + context.argument( + 'properties', + options_list=['--properties'], + help="Optional custom properties for the artifact in 'key1=value1;key2=value2' format." + ) diff --git a/azure-devops/azext_devops/dev/pipelines/pipeline.py b/azure-devops/azext_devops/dev/pipelines/pipeline.py index e856fb8f..8918e221 100644 --- a/azure-devops/azext_devops/dev/pipelines/pipeline.py +++ b/azure-devops/azext_devops/dev/pipelines/pipeline.py @@ -6,6 +6,7 @@ from webbrowser import open_new from knack.log import get_logger from knack.util import CLIError +import json from azext_devops.dev.common.services import (get_build_client, get_git_client, resolve_instance_and_project, @@ -172,7 +173,10 @@ def pipeline_run(id=None, branch=None, commit_id=None, name=None, open=False, va build = Build(definition=definition_reference, source_branch=branch, source_version=commit_id) param_variables = set_param_variable(variables) - build.parameters = param_variables + if param_variables: + build.parameters = json.dumps(param_variables) + else: + build.parameters = None queued_build = client.queue_build(build=build, project=project) diff --git a/azure-devops/azext_devops/dev/pipelines/runs_artifacts.py b/azure-devops/azext_devops/dev/pipelines/runs_artifacts.py index f490364c..deaf99d0 100644 --- a/azure-devops/azext_devops/dev/pipelines/runs_artifacts.py +++ b/azure-devops/azext_devops/dev/pipelines/runs_artifacts.py @@ -9,6 +9,9 @@ from azext_devops.dev.common.artifacttool import ArtifactToolInvoker from azext_devops.dev.common.artifacttool_updater import ArtifactToolUpdater from azext_devops.dev.common.external_tool import ProgressReportingExternalToolInvoker +import os +import time +from typing import Optional, Callable, Any logger = get_logger(__name__) @@ -39,16 +42,90 @@ def run_artifact_list(run_id, organization=None, project=None, detect=None): return artifacts -def run_artifact_upload(run_id, artifact_name, path, organization=None, project=None, detect=None): - """ Upload a pipeline artifact. +def run_artifact_upload( + run_id: int, + artifact_name: str, + path: str, + organization: Optional[str] = None, + project: Optional[str] = None, + detect: Optional[bool] = None, + properties: Optional[str] = None, + validate_path: bool = True, + dry_run: bool = False, + retry_count: int = 3, + backoff_seconds: float = 0.1, + log_progress: bool = True, + on_progress: Optional[Callable[[str, int], None]] = None, + on_complete: Optional[Callable[[Any], None]] = None, + on_error: Optional[Callable[[Exception], None]] = None, +) -> Any: + """ + Upload a pipeline artifact with robust options and hooks. + :param run_id: ID of the run that the artifact is associated to. :type run_id: int :param artifact_name: Name of the artifact to upload. :type artifact_name: string :param path: Path to upload the artifact from. :type path: string + :param properties: Optional custom properties for the artifact in 'key1=value1;key2=value2' format. + :type properties: string + :param validate_path: Whether to validate the path before upload. + :type validate_path: bool + :param dry_run: If True, do not actually upload. + :type dry_run: bool + :param retry_count: Number of times to retry on failure. + :type retry_count: int + :param log_progress: Whether to log progress. + :type log_progress: bool + :param on_progress: Optional callback for progress updates. + :type on_progress: callable + :param on_complete: Optional callback for completion. + :type on_complete: callable + :param on_error: Optional callback for errors. + :type on_error: callable """ organization, project = resolve_instance_and_project(detect=detect, organization=organization, project=project) + + if validate_path and not os.path.exists(path): + error_msg = f"Artifact path does not exist: {path}" + logger.error(error_msg) + exc = FileNotFoundError(error_msg) + if on_error: + on_error(exc) + raise exc + + if dry_run: + logger.info("Dry run enabled. Skipping upload.") + result = {"status": "dry_run", "artifact_name": artifact_name, "path": path} + if on_complete: + on_complete(result) + return result + artifact_tool = ArtifactToolInvoker(ProgressReportingExternalToolInvoker(), ArtifactToolUpdater()) - return artifact_tool.upload_pipeline_artifact( - organization=organization, project=project, run_id=run_id, artifact_name=artifact_name, path=path) + + for attempt in range(1, retry_count + 1): + try: + logger.info(f"Upload attempt {attempt} of {retry_count}") + result = artifact_tool.upload_pipeline_artifact( + organization=organization, + project=project, + run_id=run_id, + artifact_name=artifact_name, + path=path, + properties=properties, + ) + if log_progress and on_progress: + on_progress(f"Upload completed for {artifact_name}", 100) + if on_complete: + on_complete(result) + return result + except Exception as ex: + logger.error(f"Upload attempt {attempt} failed: {ex}") + if on_error: + on_error(ex) + if attempt < retry_count: + time.sleep(backoff_seconds * attempt) + else: + logger.error("All upload attempts failed.") + raise ex diff --git a/azure-devops/azext_devops/tests/latest/pipelines/test_pipeline_runs_artifacts.py b/azure-devops/azext_devops/tests/latest/pipelines/test_pipeline_runs_artifacts.py index ff1889f4..e71aee9f 100644 --- a/azure-devops/azext_devops/tests/latest/pipelines/test_pipeline_runs_artifacts.py +++ b/azure-devops/azext_devops/tests/latest/pipelines/test_pipeline_runs_artifacts.py @@ -76,11 +76,20 @@ def test_runs_artifacts_download(self): 'Downloading') def test_runs_artifacts_upload(self): - # set return values response = run_artifact_upload( - run_id=12345, artifact_name=self._TEST_ARTIFACT_NAME, path=self._TEST_ARTIFACT_PATH, - organization=self._TEST_DEVOPS_ORGANIZATION, project=self._TEST_DEVOPS_PROJECT, detect=None) - #assert + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + detect=None, + validate_path=False, + retry_count=2, + log_progress=False, + on_progress=None, + on_complete=None, + on_error=None, + ) self.mock_run_artifacttool.assert_called_with(self._TEST_DEVOPS_ORGANIZATION, [ 'pipelineartifact', @@ -94,5 +103,212 @@ def test_runs_artifacts_upload(self): ], 'Uploading') + def test_runs_artifacts_upload_with_properties(self): + properties = "user-key1=value1;user-key2=value2" + response = run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + detect=None, + properties=properties, + validate_path=False, + retry_count=2, + log_progress=False, + on_progress=None, + on_complete=None, + on_error=None, + ) + self.mock_run_artifacttool.assert_called_with(self._TEST_DEVOPS_ORGANIZATION, + [ + 'pipelineartifact', + 'publish', + '--service', self._TEST_DEVOPS_ORGANIZATION, + '--patvar', ARTIFACTTOOL_PAT_ENVKEY, + '--project', self._TEST_DEVOPS_PROJECT, + '--pipeline-id', 12345, + '--artifact-name', self._TEST_ARTIFACT_NAME, + '--path', self._TEST_ARTIFACT_PATH, + '--properties', properties + ], + 'Uploading') + self.assertTrue(self.mock_run_artifacttool.called, "ArtifactTool upload was not called with properties.") + + def test_artifacttoolinvoker_upload_pipeline_artifact_properties(self): + # Directly test ArtifactToolInvoker.upload_pipeline_artifact handles properties + from azext_devops.dev.common.artifacttool import ArtifactToolInvoker + mock_invoker = ArtifactToolInvoker(self.mock_run_artifacttool, None) + organization = self._TEST_DEVOPS_ORGANIZATION + project = self._TEST_DEVOPS_PROJECT + run_id = 12345 + artifact_name = self._TEST_ARTIFACT_NAME + path = self._TEST_ARTIFACT_PATH + properties = "user-key1=value1;user-key2=value2" + # Patch run_artifacttool to capture args + with patch.object(mock_invoker, "run_artifacttool", return_value="ok") as mock_run: + result = mock_invoker.upload_pipeline_artifact( + organization=organization, + project=project, + run_id=run_id, + artifact_name=artifact_name, + path=path, + properties=properties + ) + called_args = mock_run.call_args[0][1] + assert "--properties" in called_args + assert properties in called_args + assert result == "ok" + + def test_help_documentation_updated(self): + # Check help text for --properties and that an example is present (not enforcing exact example string) + import os + help_file = os.path.join( + os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__)))), + "dev", "pipelines", "_help.py" + ) + if not os.path.isfile(help_file): + import warnings + warnings.warn(f"_help.py not found at expected location: {help_file}; skipping help documentation content check.") + return + with open(help_file, encoding="utf-8") as f: + help_content = f.read() + assert "--properties" in help_content + assert "--properties" in help_content and "example" in help_content.lower() + def test_runs_artifacts_upload_dry_run(self): + # dry_run should prevent actual upload + result = run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + dry_run=True, + validate_path=False + ) + self.assertEqual(result["status"], "dry_run") + self.assertFalse(self.mock_run_artifacttool.called, "ArtifactTool upload should not be called in dry_run mode.") + + def test_runs_artifacts_upload_validate_path(self): + # Should raise FileNotFoundError if path does not exist and validate_path is True + import os + from azext_devops.dev.pipelines.runs_artifacts import run_artifact_upload + invalid_path = "Z:/nonexistent" + try: + run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=invalid_path, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + validate_path=True + ) + self.fail("Expected FileNotFoundError for invalid path") + except FileNotFoundError: + pass + + def test_runs_artifacts_upload_retry_count(self): + # Simulate failure and verify retry logic + from azext_devops.dev.pipelines.runs_artifacts import run_artifact_upload + self.mock_run_artifacttool.side_effect = Exception("Simulated failure") + try: + run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + retry_count=2, + validate_path=False + ) + self.fail("Expected Exception after retries") + except Exception: + self.assertGreater(self.mock_run_artifacttool.call_count, 1) + + def test_runs_artifacts_upload_callbacks(self): + # Verify on_progress, on_complete, on_error are called + progress_called = [] + complete_called = [] + error_called = [] + + def on_progress(msg, percent): + progress_called.append((msg, percent)) + + def on_complete(result): + complete_called.append(result) + + def on_error(err): + error_called.append(str(err)) + + # Success case + self.mock_run_artifacttool.return_value = "success" + run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + on_progress=on_progress, + on_complete=on_complete, + on_error=on_error, + validate_path=False + ) + self.assertTrue(any("Upload completed" in msg for msg, _ in progress_called)) + self.assertIn("success", complete_called) + + # Failure case + self.mock_run_artifacttool.side_effect = Exception("fail") + try: + run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + on_progress=on_progress, + on_complete=on_complete, + on_error=on_error, + retry_count=1, + validate_path=False + ) + except Exception: + pass + self.assertTrue(any("fail" in err for err in error_called)) + + def test_runs_artifacts_upload_log_progress(self): + # log_progress disables/enables progress callback + progress_called = [] + + def on_progress(msg, percent): + progress_called.append((msg, percent)) + + self.mock_run_artifacttool.return_value = "success" + # log_progress True: callback should be called + run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + on_progress=on_progress, + log_progress=True, + validate_path=False + ) + self.assertTrue(progress_called) + + # log_progress False: callback should not be called + progress_called.clear() + run_artifact_upload( + run_id=12345, + artifact_name=self._TEST_ARTIFACT_NAME, + path=self._TEST_ARTIFACT_PATH, + organization=self._TEST_DEVOPS_ORGANIZATION, + project=self._TEST_DEVOPS_PROJECT, + on_progress=on_progress, + log_progress=False, + validate_path=False + ) + self.assertFalse(progress_called) + if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main()