From b1fd866bb60a8e84f737210854ad121d912b5fcc Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" <19791+drernie@users.noreply.github.com> Date: Fri, 27 Dec 2024 15:08:27 -0800 Subject: [PATCH 1/5] wrap push_manifest --- api/python/quilt3/backends/base.py | 17 +++++++------- api/python/quilt3/packages.py | 7 +++--- api/python/tests/integration/test_packages.py | 23 ++++++++++++------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/api/python/quilt3/backends/base.py b/api/python/quilt3/backends/base.py index ff8a065deec..dfc5548d055 100644 --- a/api/python/quilt3/backends/base.py +++ b/api/python/quilt3/backends/base.py @@ -87,7 +87,8 @@ def delete_package_version(self, pkg_name: str, top_hash: str): pass @abc.abstractmethod - def push_manifest(self, pkg_name: str, top_hash: str, manifest_data: bytes): + def push_manifest(self, pkg_name: str, top_hash: str, manifest_data: bytes, + put_options: dict = None): pass @abc.abstractmethod @@ -134,14 +135,14 @@ def manifests_package_dir(self, pkg_name: str) -> PhysicalKey: def manifest_pk(self, pkg_name: str, top_hash: str) -> PhysicalKey: return self.root.join(f'packages/{top_hash}') - def push_manifest(self, pkg_name: str, top_hash: str, manifest_data: bytes): + def push_manifest(self, pkg_name: str, top_hash: str, manifest_data: bytes, put_options: dict = None): """returns: timestamp to support catalog drag-and-drop => browse""" - put_bytes(manifest_data, self.manifest_pk(pkg_name, top_hash)) + put_bytes(manifest_data, self.manifest_pk(pkg_name, top_hash), put_options=put_options) hash_bytes = top_hash.encode() # TODO: use a float to string formatter instead of double casting timestamp_str = str(int(time.time())) - put_bytes(hash_bytes, self.pointer_pk(pkg_name, timestamp_str)) - put_bytes(hash_bytes, self.pointer_latest_pk(pkg_name)) + put_bytes(hash_bytes, self.pointer_pk(pkg_name, timestamp_str), put_options=put_options) + put_bytes(hash_bytes, self.pointer_latest_pk(pkg_name), put_options=put_options) return timestamp_str @staticmethod @@ -246,9 +247,9 @@ def list_package_versions(self, pkg_name: str): for dt, top_hash in self.list_package_versions_with_timestamps(pkg_name): yield str(int(dt.timestamp())), top_hash - def push_manifest(self, pkg_name: str, top_hash: str, manifest_data: bytes): - put_bytes(manifest_data, self.manifest_pk(pkg_name, top_hash)) - put_bytes(top_hash.encode(), self.pointer_latest_pk(pkg_name)) + def push_manifest(self, pkg_name: str, top_hash: str, manifest_data: bytes, put_options: dict = None): + put_bytes(manifest_data, self.manifest_pk(pkg_name, top_hash), put_options=put_options) + put_bytes(top_hash.encode(), self.pointer_latest_pk(pkg_name), put_options=put_options) @staticmethod def _top_hash_from_path(path: str) -> str: diff --git a/api/python/quilt3/packages.py b/api/python/quilt3/packages.py index 647db51600c..b44623d3854 100644 --- a/api/python/quilt3/packages.py +++ b/api/python/quilt3/packages.py @@ -1064,6 +1064,7 @@ def build(self, name, registry=None, message=None, *, workflow=...): registry: registry to build to defaults to local registry message: the commit message of the package + put_options: optional arguments to pass to the PutObject operation %(workflow)s Returns: @@ -1084,10 +1085,10 @@ def _build(self, name, registry, message): self._push_manifest(name, registry, top_hash) return top_hash - def _push_manifest(self, name, registry, top_hash): + def _push_manifest(self, name, registry, top_hash, put_options=None): manifest = io.BytesIO() self._dump(manifest) - registry.push_manifest(name, top_hash, manifest.getvalue()) + registry.push_manifest(name, top_hash, manifest.getvalue(), put_options=put_options) @ApiTelemetry("package.dump") def dump(self, writable_file): @@ -1583,7 +1584,7 @@ def physical_key_is_temp_file(pk): latest_hash = get_latest_hash() check_hash_conficts(latest_hash) - pkg._push_manifest(name, registry, top_hash) + pkg._push_manifest(name, registry, top_hash, put_options=put_options) if print_info: shorthash = registry.shorten_top_hash(name, top_hash) diff --git a/api/python/tests/integration/test_packages.py b/api/python/tests/integration/test_packages.py index 58ebd7169c4..97cd31f0fd0 100644 --- a/api/python/tests/integration/test_packages.py +++ b/api/python/tests/integration/test_packages.py @@ -1260,6 +1260,7 @@ def test_commit_message_on_push(self, mocked_workflow_validate): 'Quilt/test_pkg_name', registry, mock.sentinel.top_hash, + put_options=None, ) mocked_workflow_validate.assert_called_once_with( registry=registry, @@ -1923,10 +1924,11 @@ def test_push_dest_fn(self): ) push_manifest_mock = self.patch_s3_registry('push_manifest') self.patch_s3_registry('shorten_top_hash', return_value='7a67ff4') - pkg.push(pkg_name, registry='s3://test-bucket', dest=dest_fn, force=True) + pkg.push(pkg_name, registry='s3://test-bucket', dest=dest_fn, force=True, +) dest_fn.assert_called_once_with(lk, pkg[lk]) - push_manifest_mock.assert_called_once_with(pkg_name, mock.sentinel.top_hash, ANY) + push_manifest_mock.assert_called_once_with(pkg_name, mock.sentinel.top_hash, ANY, put_options=None) assert Package.load( BytesIO(push_manifest_mock.call_args[0][2]) )[lk].physical_key == PhysicalKey(dest_bucket, dest_key, version) @@ -1952,7 +1954,7 @@ def test_push_selector_fn_false(self): selector_fn.assert_called_once_with(lk, pkg[lk]) calculate_checksum_mock.assert_called_once_with([PhysicalKey(src_bucket, src_key, src_version)], [0]) - push_manifest_mock.assert_called_once_with(pkg_name, mock.sentinel.top_hash, ANY) + push_manifest_mock.assert_called_once_with(pkg_name, mock.sentinel.top_hash, ANY, put_options=None) assert Package.load( BytesIO(push_manifest_mock.call_args[0][2]) )[lk].physical_key == PhysicalKey(src_bucket, src_key, src_version) @@ -1999,7 +2001,7 @@ def test_push_selector_fn_true(self): selector_fn.assert_called_once_with(lk, pkg[lk]) calculate_checksum_mock.assert_called_once_with([], []) - push_manifest_mock.assert_called_once_with(pkg_name, mock.sentinel.top_hash, ANY) + push_manifest_mock.assert_called_once_with(pkg_name, mock.sentinel.top_hash, ANY, put_options=None) assert Package.load( BytesIO(push_manifest_mock.call_args[0][2]) )[lk].physical_key == PhysicalKey(dst_bucket, dst_key, dst_version) @@ -2047,10 +2049,15 @@ class PackageTestV2(PackageTest): def local_manifest_timestamp_fixer(self, timestamp): wrapped = self.LocalPackageRegistryDefault.push_manifest - def wrapper(pkg_registry, pkg_name, top_hash, manifest_data): - wrapped(pkg_registry, pkg_name, top_hash, manifest_data) - os.utime(pkg_registry._manifest_parent_pk(pkg_name, top_hash).path, (timestamp, timestamp)) - return patch.object(self.LocalPackageRegistryDefault, 'push_manifest', wrapper) + def wrapper(pkg_registry, pkg_name, top_hash, manifest_data, put_options=None): + wrapped(pkg_registry, pkg_name, top_hash, manifest_data, put_options=put_options) + os.utime( + pkg_registry._manifest_parent_pk(pkg_name, top_hash).path, + (timestamp, timestamp) + ) + return patch.object( + self.LocalPackageRegistryDefault, 'push_manifest', wrapper + ) def _test_list_remote_packages_setup_stubber(self, pkg_registry, *, pkg_names): self.s3_stubber.add_response( From c2a341da434ebaa2da906901cb5363976d918bc8 Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" <19791+drernie@users.noreply.github.com> Date: Fri, 27 Dec 2024 15:10:48 -0800 Subject: [PATCH 2/5] add put_options to build --- api/python/quilt3/packages.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/python/quilt3/packages.py b/api/python/quilt3/packages.py index b44623d3854..ef48a30f11b 100644 --- a/api/python/quilt3/packages.py +++ b/api/python/quilt3/packages.py @@ -1055,7 +1055,7 @@ def _validate_with_workflow(self, *, registry, workflow, name, message): @ApiTelemetry("package.build") @_fix_docstring(workflow=_WORKFLOW_PARAM_DOCSTRING) - def build(self, name, registry=None, message=None, *, workflow=...): + def build(self, name, registry=None, message=None, *, workflow=..., put_options=None): """ Serializes this package to a registry. @@ -1064,17 +1064,17 @@ def build(self, name, registry=None, message=None, *, workflow=...): registry: registry to build to defaults to local registry message: the commit message of the package - put_options: optional arguments to pass to the PutObject operation %(workflow)s + put_options: optional arguments to pass to the PutObject operation Returns: The top hash as a string. """ registry = get_package_registry(registry) self._validate_with_workflow(registry=registry, workflow=workflow, name=name, message=message) - return self._build(name=name, registry=registry, message=message) + return self._build(name=name, registry=registry, message=message, put_options=put_options) - def _build(self, name, registry, message): + def _build(self, name, registry, message, put_options=None): validate_package_name(name) registry = get_package_registry(registry) @@ -1082,7 +1082,7 @@ def _build(self, name, registry, message): self._calculate_missing_hashes() top_hash = self.top_hash - self._push_manifest(name, registry, top_hash) + self._push_manifest(name, registry, top_hash, put_options=put_options) return top_hash def _push_manifest(self, name, registry, top_hash, put_options=None): From cf019c09316a15e29a2d08d9ec4b02554a487e6c Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" <19791+drernie@users.noreply.github.com> Date: Fri, 27 Dec 2024 15:14:46 -0800 Subject: [PATCH 3/5] _mock_package_build --- lambdas/pkgpush/tests/test_index.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lambdas/pkgpush/tests/test_index.py b/lambdas/pkgpush/tests/test_index.py index 7f375999408..58b93295089 100644 --- a/lambdas/pkgpush/tests/test_index.py +++ b/lambdas/pkgpush/tests/test_index.py @@ -587,9 +587,11 @@ def make_request(self, params, **kwargs): ) @contextlib.contextmanager - def _mock_package_build(self, entries, *, message=..., expected_workflow=...): + def _mock_package_build(self, entries, *, message=..., expected_workflow=..., put_options=None): if message is ...: message = self.dst_commit_message + if put_options is None: + put_options = {} # Use a test package to verify manifest entries test_pkg = Package() @@ -614,11 +616,6 @@ def _mock_package_build(self, entries, *, message=..., expected_workflow=...): self.s3_stubber.add_response( 'put_object', service_response={}, - expected_params={ - 'Body': manifest.read(), - 'Bucket': self.dst_bucket, - 'Key': f'.quilt/packages/{test_pkg.top_hash}', - }, ) self.s3_stubber.add_response( 'put_object', @@ -627,15 +624,17 @@ def _mock_package_build(self, entries, *, message=..., expected_workflow=...): 'Body': str.encode(test_pkg.top_hash), 'Bucket': self.dst_bucket, 'Key': f'.quilt/named_packages/{self.dst_pkg_name}/{str(int(self.mock_timestamp))}', + **put_options, }, ) self.s3_stubber.add_response( - 'put_object', + "put_object", service_response={}, expected_params={ - 'Body': str.encode(test_pkg.top_hash), - 'Bucket': self.dst_bucket, - 'Key': f'.quilt/named_packages/{self.dst_pkg_name}/latest', + "Body": str.encode(test_pkg.top_hash), + "Bucket": self.dst_bucket, + "Key": f".quilt/named_packages/{self.dst_pkg_name}/latest", + **put_options, }, ) with mock.patch('quilt3.workflows.validate', return_value=mocked_workflow_data) as workflow_validate_mock: From 55cfa7609895da39821233935e74870df51d3c0b Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" <19791+drernie@users.noreply.github.com> Date: Fri, 27 Dec 2024 15:16:48 -0800 Subject: [PATCH 4/5] lint --- api/python/tests/integration/test_packages.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/python/tests/integration/test_packages.py b/api/python/tests/integration/test_packages.py index 97cd31f0fd0..e2e236ebbed 100644 --- a/api/python/tests/integration/test_packages.py +++ b/api/python/tests/integration/test_packages.py @@ -1924,8 +1924,7 @@ def test_push_dest_fn(self): ) push_manifest_mock = self.patch_s3_registry('push_manifest') self.patch_s3_registry('shorten_top_hash', return_value='7a67ff4') - pkg.push(pkg_name, registry='s3://test-bucket', dest=dest_fn, force=True, -) + pkg.push(pkg_name, registry='s3://test-bucket', dest=dest_fn, force=True) dest_fn.assert_called_once_with(lk, pkg[lk]) push_manifest_mock.assert_called_once_with(pkg_name, mock.sentinel.top_hash, ANY, put_options=None) From 98036c1bae6825d11bb1acb9f2405fa446a8ab7d Mon Sep 17 00:00:00 2001 From: "Dr. Ernie Prabhakar" <19791+drernie@users.noreply.github.com> Date: Fri, 27 Dec 2024 15:22:17 -0800 Subject: [PATCH 5/5] Package.build gendoc --- docs/api-reference/Package.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/api-reference/Package.md b/docs/api-reference/Package.md index 0c1f8f94b17..6ef8f02e914 100644 --- a/docs/api-reference/Package.md +++ b/docs/api-reference/Package.md @@ -194,7 +194,7 @@ no such entry exists. Sets user metadata on this Package. -## Package.build(self, name, registry=None, message=None, \*, workflow=Ellipsis) {#Package.build} +## Package.build(self, name, registry=None, message=None, \*, workflow=Ellipsis, put\_options=None) {#Package.build} Serializes this package to a registry. @@ -208,6 +208,8 @@ __Arguments__ If not specified, the default workflow will be used. * __For details see__: https://docs.quiltdata.com/advanced-usage/workflows +* __put_options__: optional arguments to pass to the PutObject operation + __Returns__