From d796e879de409e523ff87806071b0ff2f1ae3273 Mon Sep 17 00:00:00 2001 From: Kyle-Neale Date: Thu, 8 May 2025 14:35:03 -0400 Subject: [PATCH 1/2] Use workflow ID in wheel upload --- .builders/tests/test_upload.py | 78 +++++++++++++++++++++++++--------- .builders/upload.py | 13 ++---- 2 files changed, 63 insertions(+), 28 deletions(-) diff --git a/.builders/tests/test_upload.py b/.builders/tests/test_upload.py index d1ca345947edd..51b86971439a7 100644 --- a/.builders/tests/test_upload.py +++ b/.builders/tests/test_upload.py @@ -8,6 +8,11 @@ import upload +@pytest.fixture +def workflow_id(): + return '1234567890' + + @pytest.fixture def setup_fake_bucket(monkeypatch): """Patch google storage functions to simulate a bucket.""" @@ -89,13 +94,6 @@ def fake_hash(path: Path): return _setup_hash -@pytest.fixture -def frozen_timestamp(monkeypatch): - timestamp = 20241327_090504 - monkeypatch.setattr(upload, 'timestamp_build_number', mock.Mock(return_value=timestamp)) - return timestamp - - def test_upload_external(setup_targets_dir, setup_fake_bucket): wheels = { 'external': [ @@ -114,7 +112,7 @@ def test_upload_external(setup_targets_dir, setup_fake_bucket): } bucket, uploads = setup_fake_bucket(bucket_files) - upload.upload(targets_dir) + upload.upload(targets_dir, workflow_id) bucket_files = [f.name for f in bucket.list_blobs()] assert 'external/all-new/all_new-2.31.0-py3-none-any.whl' in bucket_files @@ -124,7 +122,7 @@ def test_upload_external(setup_targets_dir, setup_fake_bucket): assert {'all_new-2.31.0-py3-none-any.whl', 'updated_version-3.14.1-cp311-cp311-manylinux1_x86_64.whl'} <= uploads -def test_upload_built_no_conflict(setup_targets_dir, setup_fake_bucket, frozen_timestamp): +def test_upload_built_no_conflict(setup_targets_dir, setup_fake_bucket, workflow_id): wheels = { 'built': [ ('without_collision-3.14.1-cp311-cp311-manylinux2010_x86_64.whl', 'without-collision', '3.14.1', '>=3.7'), @@ -134,11 +132,11 @@ def test_upload_built_no_conflict(setup_targets_dir, setup_fake_bucket, frozen_t bucket, uploads = setup_fake_bucket({}) - upload.upload(targets_dir) + upload.upload(targets_dir, workflow_id) bucket_files = [f.name for f in bucket.list_blobs()] assert ( - f'built/without-collision/without_collision-3.14.1-{frozen_timestamp}-cp311-cp311-manylinux2010_x86_64.whl' + f'built/without-collision/without_collision-3.14.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files ) @@ -147,6 +145,7 @@ def test_upload_built_existing_sha_match_does_not_upload( setup_targets_dir, setup_fake_bucket, setup_fake_hash, + workflow_id, ): whl_hash = 'some-hash' @@ -167,7 +166,7 @@ def test_upload_built_existing_sha_match_does_not_upload( 'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl': whl_hash, }) - upload.upload(targets_dir) + upload.upload(targets_dir, workflow_id) assert not uploads @@ -176,7 +175,7 @@ def test_upload_built_existing_different_sha_does_upload( setup_targets_dir, setup_fake_bucket, setup_fake_hash, - frozen_timestamp, + workflow_id, ): original_hash = 'first-hash' new_hash = 'second-hash' @@ -198,20 +197,21 @@ def test_upload_built_existing_different_sha_does_upload( 'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl': new_hash, }) - upload.upload(targets_dir) + upload.upload(targets_dir, workflow_id) uploads = {str(Path(f).name) for f in uploads} assert uploads == {'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl'} bucket_files = {f.name for f in bucket.list_blobs()} - assert f'built/existing/existing-1.1.1-{frozen_timestamp}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files + assert f'built/existing/existing-1.1.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files def test_upload_built_existing_sha_match_does_not_upload_multiple_existing_builds( setup_targets_dir, setup_fake_bucket, setup_fake_hash, + workflow_id, ): matching_hash = 'some-hash' non_matching_hash = 'xxxx' @@ -242,7 +242,7 @@ def test_upload_built_existing_sha_match_does_not_upload_multiple_existing_build 'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl': matching_hash, }) - upload.upload(targets_dir) + upload.upload(targets_dir, workflow_id) assert not uploads @@ -251,16 +251,55 @@ def test_upload_built_existing_different_sha_does_upload_multiple_existing_build setup_targets_dir, setup_fake_bucket, setup_fake_hash, - frozen_timestamp, + workflow_id, +): + original_hash = 'first-hash' + new_hash = 'second-hash' + + wheels = { + 'built': [ + ('existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl', 'existing', '1.1.1', '>=3.7'), + ] + } + targets_dir = setup_targets_dir(wheels) + + bucket_files = { + 'built/existing/existing-1.1.1-2024132600000-cp311-cp311-manylinux2010_x86_64.whl': + {'requires-python': '', 'sha256': 'b'}, + 'built/existing/existing-1.1.1-2024132700000-cp311-cp311-manylinux2010_x86_64.whl': + {'requires-python': '', 'sha256': original_hash}, + } + bucket, uploads = setup_fake_bucket(bucket_files) + + setup_fake_hash({ + 'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl': new_hash, + }) + + upload.upload(targets_dir, workflow_id) + + uploads = {str(Path(f).name) for f in uploads} + + assert uploads == {'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl'} + + bucket_files = {f.name for f in bucket.list_blobs()} + assert f'built/existing/existing-1.1.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files + + +def test_build_tag_use_workflow_id( + setup_targets_dir, + setup_fake_bucket, + setup_fake_hash, ): original_hash = 'first-hash' new_hash = 'second-hash' + workflow_id = '1234567890' wheels = { 'built': [ ('existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl', 'existing', '1.1.1', '>=3.7'), ] } + targets_dir = setup_targets_dir(wheels) bucket_files = { @@ -269,17 +308,18 @@ def test_upload_built_existing_different_sha_does_upload_multiple_existing_build 'built/existing/existing-1.1.1-2024132700000-cp311-cp311-manylinux2010_x86_64.whl': {'requires-python': '', 'sha256': original_hash}, } + bucket, uploads = setup_fake_bucket(bucket_files) setup_fake_hash({ 'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl': new_hash, }) - upload.upload(targets_dir) + upload.upload(targets_dir, workflow_id) uploads = {str(Path(f).name) for f in uploads} assert uploads == {'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl'} bucket_files = {f.name for f in bucket.list_blobs()} - assert f'built/existing/existing-1.1.1-{frozen_timestamp}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files + assert f'built/existing/existing-1.1.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files \ No newline at end of file diff --git a/.builders/upload.py b/.builders/upload.py index 4a39e4bdd0655..40d0accfeffe9 100644 --- a/.builders/upload.py +++ b/.builders/upload.py @@ -58,11 +58,6 @@ def display_message_block(message: str) -> None: print(divider) -def timestamp_build_number() -> int: - """Produce a numeric timestamp to use as build numbers""" - return int(time.strftime('%Y%m%d%H%M%S')) - - def hash_file(path: Path) -> str: """Calculate the hash of the file pointed at by `path`""" with path.open('rb') as f: @@ -95,7 +90,7 @@ def _build_number_of_wheel_blob(wheel_path: Blob) -> int: return int(build_number[0]) if build_number else -1 -def upload(targets_dir): +def upload(targets_dir, workflow_id): client = storage.Client() bucket = client.bucket(BUCKET_NAME) artifact_types: set[str] = set() @@ -148,8 +143,7 @@ def upload(targets_dir): 'with the same hash') continue - build_number = timestamp_build_number() - artifact_name = f'{name}-{version}-{build_number}-{python_tag}-{abi_tag}-{platform_tag}.whl' + artifact_name = f'{name}-{version}-{workflow_id}-{python_tag}-{abi_tag}-{platform_tag}.whl' artifact = bucket.blob(f'{artifact_type}/{project_name}/{artifact_name}') print(f'{padding}Artifact: {artifact_name}') @@ -213,5 +207,6 @@ def upload(targets_dir): if __name__ == '__main__': parser = argparse.ArgumentParser(prog='builder', allow_abbrev=False) parser.add_argument('targets_dir') + parser.add_argument('workflow_id') args = parser.parse_args() - upload(args.targets_dir) + upload(args.targets_dir, args.workflow_id) From 905f541714dd437f938627f0275e8857b8024467 Mon Sep 17 00:00:00 2001 From: Kyle-Neale Date: Wed, 14 May 2025 09:25:40 -0400 Subject: [PATCH 2/2] Use workflow ID in wheel upload --- .builders/tests/test_upload.py | 8 ++++---- .builders/upload.py | 13 +++++++++---- .github/workflows/resolve-build-deps.yaml | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/.builders/tests/test_upload.py b/.builders/tests/test_upload.py index 51b86971439a7..4636b97ccbdae 100644 --- a/.builders/tests/test_upload.py +++ b/.builders/tests/test_upload.py @@ -136,7 +136,7 @@ def test_upload_built_no_conflict(setup_targets_dir, setup_fake_bucket, workflow bucket_files = [f.name for f in bucket.list_blobs()] assert ( - f'built/without-collision/without_collision-3.14.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' + f'built/without-collision/without_collision-3.14.1-{workflow_id}WID-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files ) @@ -204,7 +204,7 @@ def test_upload_built_existing_different_sha_does_upload( assert uploads == {'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl'} bucket_files = {f.name for f in bucket.list_blobs()} - assert f'built/existing/existing-1.1.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files + assert f'built/existing/existing-1.1.1-{workflow_id}WID-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files def test_upload_built_existing_sha_match_does_not_upload_multiple_existing_builds( @@ -282,7 +282,7 @@ def test_upload_built_existing_different_sha_does_upload_multiple_existing_build assert uploads == {'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl'} bucket_files = {f.name for f in bucket.list_blobs()} - assert f'built/existing/existing-1.1.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files + assert f'built/existing/existing-1.1.1-{workflow_id}WID-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files def test_build_tag_use_workflow_id( @@ -322,4 +322,4 @@ def test_build_tag_use_workflow_id( assert uploads == {'existing-1.1.1-cp311-cp311-manylinux2010_x86_64.whl'} bucket_files = {f.name for f in bucket.list_blobs()} - assert f'built/existing/existing-1.1.1-{workflow_id}-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files \ No newline at end of file + assert f'built/existing/existing-1.1.1-{workflow_id}WID-cp311-cp311-manylinux2010_x86_64.whl' in bucket_files \ No newline at end of file diff --git a/.builders/upload.py b/.builders/upload.py index 40d0accfeffe9..4a39e4bdd0655 100644 --- a/.builders/upload.py +++ b/.builders/upload.py @@ -58,6 +58,11 @@ def display_message_block(message: str) -> None: print(divider) +def timestamp_build_number() -> int: + """Produce a numeric timestamp to use as build numbers""" + return int(time.strftime('%Y%m%d%H%M%S')) + + def hash_file(path: Path) -> str: """Calculate the hash of the file pointed at by `path`""" with path.open('rb') as f: @@ -90,7 +95,7 @@ def _build_number_of_wheel_blob(wheel_path: Blob) -> int: return int(build_number[0]) if build_number else -1 -def upload(targets_dir, workflow_id): +def upload(targets_dir): client = storage.Client() bucket = client.bucket(BUCKET_NAME) artifact_types: set[str] = set() @@ -143,7 +148,8 @@ def upload(targets_dir, workflow_id): 'with the same hash') continue - artifact_name = f'{name}-{version}-{workflow_id}-{python_tag}-{abi_tag}-{platform_tag}.whl' + build_number = timestamp_build_number() + artifact_name = f'{name}-{version}-{build_number}-{python_tag}-{abi_tag}-{platform_tag}.whl' artifact = bucket.blob(f'{artifact_type}/{project_name}/{artifact_name}') print(f'{padding}Artifact: {artifact_name}') @@ -207,6 +213,5 @@ def upload(targets_dir, workflow_id): if __name__ == '__main__': parser = argparse.ArgumentParser(prog='builder', allow_abbrev=False) parser.add_argument('targets_dir') - parser.add_argument('workflow_id') args = parser.parse_args() - upload(args.targets_dir, args.workflow_id) + upload(args.targets_dir) diff --git a/.github/workflows/resolve-build-deps.yaml b/.github/workflows/resolve-build-deps.yaml index 6d519429acfef..bc02da7f9b666 100644 --- a/.github/workflows/resolve-build-deps.yaml +++ b/.github/workflows/resolve-build-deps.yaml @@ -276,7 +276,7 @@ jobs: workload_identity_provider: projects/574011472402/locations/global/workloadIdentityPools/github/providers/integrations-core - name: Upload wheels - run: python .builders/upload.py targets + run: python .builders/upload.py targets ${{ github.run_id }} - name: Lock files run: python .builders/lock.py targets