Use workflow ID in wheel upload

Author: Kyle-Neale

.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

.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)