From 2d5bd64a426f8dea662a1055c19261ad41a0bb11 Mon Sep 17 00:00:00 2001 From: jayhawk-commits Date: Thu, 26 Feb 2026 19:43:07 -0500 Subject: [PATCH 1/4] Add optional S3 artifacts features: bucket override, skip manifest, CloudFront summary URLs - Optional artifacts bucket (S3_BUCKET_ARTIFACTS) for private workflows that should not hardcode bucket names in the script. - Optional --skip-manifest to skip the manifest upload step (default: do not skip). - Optional --s3-subdir-artifacts, --s3-summary-base-url, --s3-summary-path so job summary links can use CloudFront URLs instead of raw S3, with a path layout that matches prerelease CloudFront for tarballs and wheels. Keeps summary URLs behind secured CloudFront and avoids exposing raw S3 URLs. Made-with: Cursor --- .../github_actions/github_actions_utils.py | 17 +++++ .../github_actions/post_build_upload.py | 64 +++++++++++++++++-- 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/build_tools/github_actions/github_actions_utils.py b/build_tools/github_actions/github_actions_utils.py index 6dad164412e..b145c27c71f 100644 --- a/build_tools/github_actions/github_actions_utils.py +++ b/build_tools/github_actions/github_actions_utils.py @@ -538,6 +538,14 @@ def retrieve_bucket_info( If neither is provided, |is_pr_from_fork| is populated from the IS_PR_FROM_FORK environment variable. + Environment (optional override): + S3_BUCKET_ARTIFACTS: When set, use this bucket for the artifacts bucket + instead of the default chosen from repository/fork/release_type. + + Security: Bucket permissions are enforced by AWS IAM, not by this code. + When using S3_BUCKET_ARTIFACTS, ensure the role assumed by the workflow + (e.g. via OIDC) is scoped so it can write only to intended buckets. + Returns a tuple [EXTERNAL_REPO, BUCKET], where: - EXTERNAL_REPO = if CI is run on an external repo, we create a S3 sub-folder to avoid conflicting run IDs @@ -584,6 +592,15 @@ def retrieve_bucket_info( else f"{owner}-{repo_name}/" ) + # When set, use explicit artifacts bucket. + s3_bucket_artifacts = os.getenv("S3_BUCKET_ARTIFACTS", "").strip() + if s3_bucket_artifacts: + _log(" S3_BUCKET_ARTIFACTS is set, using explicit artifacts bucket") + _log("Retrieved bucket info:") + _log(f" external_repo: {external_repo}") + _log(f" bucket : {s3_bucket_artifacts}") + return (external_repo, s3_bucket_artifacts) + release_type = os.getenv("RELEASE_TYPE") if release_type: _log(f" (implicit) RELEASE_TYPE: {release_type}") diff --git a/build_tools/github_actions/post_build_upload.py b/build_tools/github_actions/post_build_upload.py index 979373575ca..503c1793712 100644 --- a/build_tools/github_actions/post_build_upload.py +++ b/build_tools/github_actions/post_build_upload.py @@ -7,6 +7,20 @@ [--build-dir BUILD_DIR] [--upload | --no-upload] (default enabled if the `CI` env var is set) [--run-id RUN_ID] + [--s3-subdir-artifacts PREFIX] [--s3-summary-base-url URL] [--s3-summary-path PREFIX] + [--skip-manifest] + +S3 path/URL arguments (--s3-subdir-artifacts, --s3-summary-base-url, --s3-summary-path): + Must not end with a forward slash. The script joins segments with "/". + --s3-subdir-artifacts is the path prefix in the S3 bucket; --s3-summary-path is + the corresponding path as it appears in the CloudFront (summary) URL. This + optional path aligns with the prerelease CloudFront layout used for tarballs + and wheels. + + Example (CloudFront base URL for job summary links): + --s3-subdir-artifacts v3/artifacts + --s3-summary-base-url https://rocm.foobar.amd.com + --s3-summary-path artifacts This script runs after building TheRock, where this script does: 1. Create log archives @@ -309,10 +323,23 @@ def run(args): external_repo_path, bucket = retrieve_bucket_info() run_id = args.run_id - bucket_uri = f"s3://{bucket}/{external_repo_path}{run_id}-{PLATFORM}" - bucket_url = ( - f"https://{bucket}.s3.amazonaws.com/{external_repo_path}{run_id}-{PLATFORM}" + path_suffix = ( + f"{args.s3_subdir_artifacts}/{external_repo_path}{run_id}-{PLATFORM}" + if args.s3_subdir_artifacts + else f"{external_repo_path}{run_id}-{PLATFORM}" ) + bucket_uri = f"s3://{bucket}/{path_suffix}" + + if args.s3_summary_base_url: + run_suffix = f"{external_repo_path}{run_id}-{PLATFORM}" + summary_path = ( + f"{args.s3_summary_path}/{run_suffix}" + if args.s3_summary_path + else path_suffix + ) + bucket_url = f"{args.s3_summary_base_url}/{summary_path}" + else: + bucket_url = f"https://{bucket}.s3.amazonaws.com/{path_suffix}" log("Write Windows time sync log") log("----------------------") @@ -328,9 +355,10 @@ def run(args): log("----------") upload_logs_to_s3(args.artifact_group, args.build_dir, bucket_uri) - log("Upload manifest") - log("----------------") - upload_manifest_to_s3(args.artifact_group, args.build_dir, bucket_uri) + if not args.skip_manifest: + log("Upload manifest") + log("----------------") + upload_manifest_to_s3(args.artifact_group, args.build_dir, bucket_uri) log("Write github actions build summary") log("--------------------") @@ -363,6 +391,30 @@ def run(args): parser.add_argument( "--job-status", type=str, help="Status of this Job ('success', 'failure')" ) + parser.add_argument( + "--s3-subdir-artifacts", + type=str, + default="", + help="S3 path prefix for artifacts (e.g. v3/artifacts). Must not end with /.", + ) + parser.add_argument( + "--s3-summary-base-url", + type=str, + default="", + help="Base URL for job summary links (e.g. CloudFront). Must not end with /. If set, summary uses this instead of raw S3 URL.", + ) + parser.add_argument( + "--s3-summary-path", + type=str, + default="", + help="Path prefix for summary URL when --s3-summary-base-url is set (e.g. artifacts). Must not end with /.", + ) + parser.add_argument( + "--skip-manifest", + action=argparse.BooleanOptionalAction, + default=False, + help="Skip manifest upload.", + ) args = parser.parse_args() # Check preconditions for provided arguments before proceeding. From 440cb8800aecb476dd98ca8cc0dbf7e33cd96e58 Mon Sep 17 00:00:00 2001 From: Joseph Macaranas Date: Thu, 19 Mar 2026 15:47:53 -0400 Subject: [PATCH 2/4] Revert unintended changes to github_actions_api.py The merge conflict resolution incorrectly introduced two blank lines before str2bool(). This file should be unchanged from main. Co-Authored-By: Claude Sonnet 4.6 --- build_tools/github_actions/github_actions_api.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/build_tools/github_actions/github_actions_api.py b/build_tools/github_actions/github_actions_api.py index 48b8453cee9..d5e6411d27c 100644 --- a/build_tools/github_actions/github_actions_api.py +++ b/build_tools/github_actions/github_actions_api.py @@ -518,8 +518,6 @@ def gha_query_recent_branch_commits( # TODO: Consider moving str2bool to a general-purpose utils module. It's useful # for GitHub Actions (YAML has fuzzy boolean handling) but is also used broadly. - - def str2bool(value: str | None) -> bool: """Convert environment variables to boolean values.""" if not value: From 9e50ddadc3a1e59b890ba233108430979eea289e Mon Sep 17 00:00:00 2001 From: Joseph Macaranas Date: Thu, 19 Mar 2026 15:53:42 -0400 Subject: [PATCH 3/4] Strip trailing slashes from path args instead of erroring Trailing slashes on --path-prefix, --summary-base-url, and --summary-path are silently stripped, so callers don't need to worry about them. Co-Authored-By: Claude Sonnet 4.6 --- build_tools/github_actions/post_build_upload.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/build_tools/github_actions/post_build_upload.py b/build_tools/github_actions/post_build_upload.py index 4acbedc37ca..5f389d92d36 100644 --- a/build_tools/github_actions/post_build_upload.py +++ b/build_tools/github_actions/post_build_upload.py @@ -18,7 +18,6 @@ [--skip-manifest] Path/URL arguments (--path-prefix, --summary-base-url, --summary-path): - Must not end with a forward slash. The script joins segments with "/". --path-prefix is the S3 key prefix for all outputs; --summary-path is the corresponding prefix in the CDN URL used for job summary links. These let private workflows place artifacts under a versioned subdirectory while @@ -409,19 +408,19 @@ def run(args): "--path-prefix", type=str, default="", - help="S3 key prefix for all outputs (e.g. 'v3/artifacts'). Must not end with /.", + help="S3 key prefix for all outputs (e.g. 'v3/artifacts').", ) parser.add_argument( "--summary-base-url", type=str, default="", - help="CDN base URL for job summary links (e.g. 'https://artifacts.example.com'). Must not end with /. When set, summary links use this instead of the raw S3 URL.", + help="CDN base URL for job summary links (e.g. 'https://artifacts.example.com'). When set, summary links use this instead of the raw S3 URL.", ) parser.add_argument( "--summary-path", type=str, default="", - help="CDN path prefix for summary links, corresponding to --path-prefix on S3 (e.g. 'artifacts'). Must not end with /.", + help="CDN path prefix for summary links, corresponding to --path-prefix on S3 (e.g. 'artifacts').", ) parser.add_argument( "--skip-manifest", @@ -433,10 +432,9 @@ def run(args): # Check preconditions for provided arguments before proceeding. - for arg_name in ("path_prefix", "summary_base_url", "summary_path"): - val = getattr(args, arg_name) - if val and val.endswith("/"): - parser.error(f"--{arg_name.replace('_', '-')} must not end with '/'") + args.path_prefix = args.path_prefix.rstrip("/") + args.summary_base_url = args.summary_base_url.rstrip("/") + args.summary_path = args.summary_path.rstrip("/") if args.upload: if not args.run_id: From 9c69a358f4fed4648048fbe4c187d9f726d6592b Mon Sep 17 00:00:00 2001 From: Joseph Macaranas Date: Mon, 6 Apr 2026 13:06:11 -0400 Subject: [PATCH 4/4] Implement missing abstractions and add tests for PR 3663 - StorageLocation.cdn_url(): remaps storage path prefix to CDN prefix - WorkflowOutputRoot.path_prefix field: prepended to all output paths - WorkflowOutputRoot.from_workflow_run(): add bucket_override/path_prefix args - Tests: TestStorageLocationCdnUrl, TestWorkflowOutputRootPathPrefix, TestWorkflowOutputRootOverrides, TestWriteGhaBuildSummarySkipManifest, TestWriteGhaBuildSummaryCdnUrl Co-Authored-By: Claude Sonnet 4.6 --- .../_therock_utils/storage_location.py | 35 ++++ .../_therock_utils/workflow_outputs.py | 27 +++- .../github_actions/post_build_upload.py | 4 +- .../tests/post_build_upload_test.py | 97 +++++++++++ build_tools/tests/workflow_outputs_test.py | 150 ++++++++++++++++++ 5 files changed, 310 insertions(+), 3 deletions(-) diff --git a/build_tools/_therock_utils/storage_location.py b/build_tools/_therock_utils/storage_location.py index b6cd282bb96..949a41d9ead 100644 --- a/build_tools/_therock_utils/storage_location.py +++ b/build_tools/_therock_utils/storage_location.py @@ -59,3 +59,38 @@ def local_path(self, staging_dir: Path) -> Path: Full path: ``{staging_dir}/{relative_path}`` """ return staging_dir / self.relative_path + + def cdn_url( + self, + base_url: str, + strip_prefix: str = "", + cdn_prefix: str = "", + ) -> str: + """CDN URL, optionally remapping the storage path prefix to a CDN prefix. + + Strips ``strip_prefix`` from the start of ``relative_path`` (if present), + then prepends ``cdn_prefix``, and finally prepends ``base_url``. + + This lets private workflows surface artifacts through a CDN whose path + layout differs from the storage key layout. For example, files stored under + ``v3/artifacts/{run_id}-linux/...`` may be served as + ``artifacts/{run_id}-linux/...`` on the CDN. + + Args: + base_url: CDN base URL without trailing slash + (e.g. ``'https://artifacts.example.com'``). + strip_prefix: Path prefix to strip from ``relative_path`` + (e.g. ``'v3/artifacts'``). No-op if empty or not matched. + cdn_prefix: CDN path prefix to prepend after stripping + (e.g. ``'artifacts'``). No-op if empty. + + Returns: + Full CDN URL (e.g. + ``'https://artifacts.example.com/artifacts/12345-linux/file.tar.xz'``). + """ + path = self.relative_path + if strip_prefix and path.startswith(f"{strip_prefix}/"): + path = path[len(strip_prefix) + 1 :] + if cdn_prefix: + path = f"{cdn_prefix}/{path}" + return f"{base_url}/{path}" diff --git a/build_tools/_therock_utils/workflow_outputs.py b/build_tools/_therock_utils/workflow_outputs.py index 0ea06668868..7c55975c2d2 100644 --- a/build_tools/_therock_utils/workflow_outputs.py +++ b/build_tools/_therock_utils/workflow_outputs.py @@ -88,6 +88,17 @@ class WorkflowOutputRoot: platform: str """Platform name ('linux' or 'windows').""" + path_prefix: str = "" + """Optional storage path prefix prepended before all output paths. + + When set (e.g. ``'v3/artifacts'``), all outputs for this run are placed + under ``{path_prefix}/{external_repo}{run_id}-{platform}/...`` instead of + the default ``{external_repo}{run_id}-{platform}/...``. + + Useful for private workflows that need to co-locate outputs with other + content under a versioned directory on the same bucket. + """ + # -- Root ------------------------------------------------------------------- @property @@ -96,7 +107,10 @@ def prefix(self) -> str: This is the common root for all outputs from this run. """ - return f"{self.external_repo}{self.run_id}-{self.platform}" + base = f"{self.external_repo}{self.run_id}-{self.platform}" + if self.path_prefix: + return f"{self.path_prefix}/{base}" + return base def root(self) -> StorageLocation: """Location for the run output root (where build artifacts live).""" @@ -236,6 +250,8 @@ def from_workflow_run( github_repository: str | None = None, workflow_run: dict | None = None, lookup_workflow_run: bool = False, + bucket_override: str | None = None, + path_prefix: str = "", ) -> "WorkflowOutputRoot": """Create from CI workflow context. @@ -255,6 +271,11 @@ def from_workflow_run( Most callers running inside their own CI workflow do not need this — environment variables suffice. Set this when looking up another repository's workflow run (e.g. fetching artifacts). + bucket_override: When set, use this S3 bucket instead of the one + determined from the repository/fork/release_type heuristics. + Bucket permissions are enforced by AWS IAM, not by this code. + path_prefix: Optional storage path prefix for all outputs under this run + (e.g. ``'v3/artifacts'``). See ``WorkflowOutputRoot.path_prefix``. """ workflow_run_id = ( run_id if lookup_workflow_run and workflow_run is None else None @@ -264,11 +285,15 @@ def from_workflow_run( workflow_run_id=workflow_run_id, workflow_run=workflow_run, ) + if bucket_override: + _log(f" bucket_override: {bucket_override!r} (overrides {bucket!r})") + bucket = bucket_override return cls( bucket=bucket, external_repo=external_repo, run_id=run_id, platform=platform, + path_prefix=path_prefix, ) @classmethod diff --git a/build_tools/github_actions/post_build_upload.py b/build_tools/github_actions/post_build_upload.py index 3df0f396ecf..0685190d4ee 100644 --- a/build_tools/github_actions/post_build_upload.py +++ b/build_tools/github_actions/post_build_upload.py @@ -227,7 +227,7 @@ def _url(loc) -> str: if summary_base_url: return loc.cdn_url( base_url=summary_base_url, - s3_prefix=output_root.path_prefix, + strip_prefix=output_root.path_prefix, cdn_prefix=summary_path, ) return loc.https_url @@ -359,7 +359,7 @@ def run(args): "--summary-path", type=str, default="", - help="CDN path prefix for summary links, corresponding to --path-prefix on S3 (e.g. 'artifacts').", + help="CDN path prefix for summary links, corresponding to --path-prefix in storage (e.g. 'artifacts').", ) parser.add_argument( "--skip-manifest", diff --git a/build_tools/github_actions/tests/post_build_upload_test.py b/build_tools/github_actions/tests/post_build_upload_test.py index bd80748b2a7..0adc752ea29 100644 --- a/build_tools/github_actions/tests/post_build_upload_test.py +++ b/build_tools/github_actions/tests/post_build_upload_test.py @@ -28,12 +28,14 @@ def _make_output_root( platform="linux", bucket="therock-ci-artifacts", external_repo="", + path_prefix="", ): return WorkflowOutputRoot( bucket=bucket, external_repo=external_repo, run_id=run_id, platform=platform, + path_prefix=path_prefix, ) @@ -363,5 +365,100 @@ def test_summary_with_external_repo(self, mock_summary): self.assertIn("Fork-TheRock/12345-linux", call) +class TestWriteGhaBuildSummarySkipManifest(unittest.TestCase): + """Tests for --skip-manifest flag in write_gha_build_summary.""" + + @mock.patch("post_build_upload.gha_append_step_summary") + def test_skip_manifest_omits_summary_link(self, mock_summary): + """--skip-manifest suppresses the manifest link in the job summary.""" + output_root = _make_output_root() + with tempfile.TemporaryDirectory() as tmp: + build_dir = Path(tmp) + post_build_upload.write_gha_build_summary( + "gfx94X-dcgpu", build_dir, output_root, "success", skip_manifest=True + ) + + calls = [c[0][0] for c in mock_summary.call_args_list] + self.assertEqual(len(calls), 2) # logs + artifacts only + for call in calls: + self.assertNotIn("manifest", call.lower()) + + @mock.patch("post_build_upload.gha_append_step_summary") + def test_manifest_present_by_default(self, mock_summary): + """Without --skip-manifest the manifest link is included.""" + output_root = _make_output_root() + with tempfile.TemporaryDirectory() as tmp: + build_dir = Path(tmp) + post_build_upload.write_gha_build_summary( + "gfx94X-dcgpu", build_dir, output_root, "success" + ) + + calls = [c[0][0] for c in mock_summary.call_args_list] + self.assertEqual(len(calls), 3) # logs + artifacts + manifest + self.assertTrue(any("manifest" in c.lower() for c in calls)) + + +class TestWriteGhaBuildSummaryCdnUrl(unittest.TestCase): + """Tests for CDN URL substitution in write_gha_build_summary.""" + + @mock.patch("post_build_upload.gha_append_step_summary") + def test_cdn_url_replaces_s3_url(self, mock_summary): + """When summary_base_url is set, all links use the CDN URL.""" + output_root = _make_output_root() + with tempfile.TemporaryDirectory() as tmp: + build_dir = Path(tmp) + post_build_upload.write_gha_build_summary( + "gfx94X-dcgpu", + build_dir, + output_root, + "success", + summary_base_url="https://artifacts.example.com", + ) + + calls = [c[0][0] for c in mock_summary.call_args_list] + for call in calls: + self.assertIn("https://artifacts.example.com", call) + self.assertNotIn("s3.amazonaws.com", call) + + @mock.patch("post_build_upload.gha_append_step_summary") + def test_cdn_url_with_path_prefix_remapping(self, mock_summary): + """strip_prefix is stripped and cdn_prefix prepended in summary URLs.""" + output_root = _make_output_root(path_prefix="v3/artifacts") + with tempfile.TemporaryDirectory() as tmp: + build_dir = Path(tmp) + post_build_upload.write_gha_build_summary( + "gfx94X-dcgpu", + build_dir, + output_root, + "success", + summary_base_url="https://artifacts.example.com", + summary_path="artifacts", + ) + + calls = [c[0][0] for c in mock_summary.call_args_list] + # Log index link should use the CDN path prefix, not the storage path prefix + self.assertIn( + "https://artifacts.example.com/artifacts/12345-linux/logs/gfx94X-dcgpu/index.html", + calls[0], + ) + for call in calls: + self.assertNotIn("v3/artifacts", call) + self.assertNotIn("s3.amazonaws.com", call) + + @mock.patch("post_build_upload.gha_append_step_summary") + def test_no_summary_base_url_uses_s3(self, mock_summary): + """Without summary_base_url, raw S3 URLs are used.""" + output_root = _make_output_root() + with tempfile.TemporaryDirectory() as tmp: + build_dir = Path(tmp) + post_build_upload.write_gha_build_summary( + "gfx94X-dcgpu", build_dir, output_root, "success" + ) + + calls = [c[0][0] for c in mock_summary.call_args_list] + for call in calls: + self.assertIn("s3.amazonaws.com", call) + + if __name__ == "__main__": unittest.main() diff --git a/build_tools/tests/workflow_outputs_test.py b/build_tools/tests/workflow_outputs_test.py index 8fa3b43e102..53acc303e51 100644 --- a/build_tools/tests/workflow_outputs_test.py +++ b/build_tools/tests/workflow_outputs_test.py @@ -506,5 +506,155 @@ def test_workflow_run_id_triggers_api_call(self): self.assertEqual(bucket, "therock-ci-artifacts") +# --------------------------------------------------------------------------- +# StorageLocation — cdn_url +# --------------------------------------------------------------------------- + + +class TestStorageLocationCdnUrl(unittest.TestCase): + def test_cdn_url_no_prefixes(self): + """With no prefixes, cdn_url is just base_url + relative_path.""" + loc = StorageLocation("my-bucket", "12345-linux/file.tar.xz") + self.assertEqual( + loc.cdn_url("https://cdn.example.com"), + "https://cdn.example.com/12345-linux/file.tar.xz", + ) + + def test_cdn_url_with_cdn_prefix_only(self): + """cdn_prefix is prepended to the path when strip_prefix is empty.""" + loc = StorageLocation("my-bucket", "12345-linux/logs/group/index.html") + self.assertEqual( + loc.cdn_url("https://cdn.example.com", cdn_prefix="artifacts"), + "https://cdn.example.com/artifacts/12345-linux/logs/group/index.html", + ) + + def test_cdn_url_strips_prefix_and_adds_cdn_prefix(self): + """strip_prefix is stripped and cdn_prefix is prepended.""" + loc = StorageLocation( + "my-bucket", "v3/artifacts/12345-linux/logs/group/index.html" + ) + self.assertEqual( + loc.cdn_url( + "https://cdn.example.com", + strip_prefix="v3/artifacts", + cdn_prefix="artifacts", + ), + "https://cdn.example.com/artifacts/12345-linux/logs/group/index.html", + ) + + def test_cdn_url_prefix_not_matched(self): + """When strip_prefix does not match, relative_path is used as-is.""" + loc = StorageLocation("my-bucket", "12345-linux/file.tar.xz") + self.assertEqual( + loc.cdn_url( + "https://cdn.example.com", + strip_prefix="v3/artifacts", + cdn_prefix="artifacts", + ), + "https://cdn.example.com/artifacts/12345-linux/file.tar.xz", + ) + + def test_cdn_url_partial_prefix_not_stripped(self): + """A partial prefix match (no trailing slash) must not be stripped.""" + loc = StorageLocation("my-bucket", "v3/artifactsfoo/12345-linux/file.tar.xz") + result = loc.cdn_url("https://cdn.example.com", strip_prefix="v3/artifacts") + # 'v3/artifacts' without a '/' should not match 'v3/artifactsfoo/...' + self.assertIn("v3/artifactsfoo", result) + + +# --------------------------------------------------------------------------- +# WorkflowOutputRoot — path_prefix field +# --------------------------------------------------------------------------- + + +class TestWorkflowOutputRootPathPrefix(unittest.TestCase): + def test_prefix_with_path_prefix(self): + root = WorkflowOutputRoot( + bucket="b", + external_repo="", + run_id="12345", + platform="linux", + path_prefix="v3/artifacts", + ) + self.assertEqual(root.prefix, "v3/artifacts/12345-linux") + + def test_prefix_with_path_prefix_and_external_repo(self): + root = WorkflowOutputRoot( + bucket="b", + external_repo="Fork-TheRock/", + run_id="12345", + platform="linux", + path_prefix="v3/artifacts", + ) + self.assertEqual(root.prefix, "v3/artifacts/Fork-TheRock/12345-linux") + + def test_prefix_without_path_prefix_unchanged(self): + """Default path_prefix='' should not affect prefix.""" + root = WorkflowOutputRoot( + bucket="b", + external_repo="", + run_id="12345", + platform="linux", + ) + self.assertEqual(root.prefix, "12345-linux") + + def test_artifact_location_includes_path_prefix(self): + root = WorkflowOutputRoot( + bucket="my-bucket", + external_repo="", + run_id="99", + platform="linux", + path_prefix="v3/artifacts", + ) + loc = root.artifact("lib.tar.xz") + self.assertEqual(loc.relative_path, "v3/artifacts/99-linux/lib.tar.xz") + self.assertEqual(loc.s3_uri, "s3://my-bucket/v3/artifacts/99-linux/lib.tar.xz") + + +# --------------------------------------------------------------------------- +# WorkflowOutputRoot — from_workflow_run with bucket_override / path_prefix +# --------------------------------------------------------------------------- + + +class TestWorkflowOutputRootOverrides(unittest.TestCase): + @mock.patch("_therock_utils.workflow_outputs._retrieve_bucket_info") + def test_bucket_override_replaces_resolved_bucket(self, mock_retrieve): + mock_retrieve.return_value = ("", "therock-ci-artifacts") + root = WorkflowOutputRoot.from_workflow_run( + run_id="12345", platform="linux", bucket_override="my-private-bucket" + ) + self.assertEqual(root.bucket, "my-private-bucket") + self.assertEqual(root.external_repo, "") + + @mock.patch("_therock_utils.workflow_outputs._retrieve_bucket_info") + def test_bucket_override_none_uses_resolved_bucket(self, mock_retrieve): + mock_retrieve.return_value = ("", "therock-ci-artifacts") + root = WorkflowOutputRoot.from_workflow_run( + run_id="12345", platform="linux", bucket_override=None + ) + self.assertEqual(root.bucket, "therock-ci-artifacts") + + @mock.patch("_therock_utils.workflow_outputs._retrieve_bucket_info") + def test_path_prefix_stored_on_root(self, mock_retrieve): + mock_retrieve.return_value = ("", "therock-ci-artifacts") + root = WorkflowOutputRoot.from_workflow_run( + run_id="12345", platform="linux", path_prefix="v3/artifacts" + ) + self.assertEqual(root.path_prefix, "v3/artifacts") + self.assertEqual(root.prefix, "v3/artifacts/12345-linux") + + @mock.patch("_therock_utils.workflow_outputs._retrieve_bucket_info") + def test_bucket_override_and_path_prefix_together(self, mock_retrieve): + mock_retrieve.return_value = ("", "therock-ci-artifacts") + root = WorkflowOutputRoot.from_workflow_run( + run_id="12345", + platform="linux", + bucket_override="my-private-bucket", + path_prefix="v3/artifacts", + ) + self.assertEqual(root.bucket, "my-private-bucket") + self.assertEqual(root.prefix, "v3/artifacts/12345-linux") + + if __name__ == "__main__": unittest.main()