Skip to content

Commit 13e0356

Browse files
eyalrothroger-zhanggvicheey
authored
feat: Add experimental flag to boost 'package' command in conjunction with similar flag for 'build' command (#7923)
Co-authored-by: Roger Zhang <ruojiazh@amazon.com> Co-authored-by: vicheey <181402101+vicheey@users.noreply.github.com>
1 parent 73b4c52 commit 13e0356

6 files changed

Lines changed: 336 additions & 41 deletions

File tree

samcli/commands/_utils/experimental.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ class ExperimentalFlag:
4646
BuildPerformance = ExperimentalEntry(
4747
"experimentalBuildPerformance", EXPERIMENTAL_ENV_VAR_PREFIX + "BUILD_PERFORMANCE"
4848
)
49+
PackagePerformance = ExperimentalEntry(
50+
"experimentalPackagePerformance", EXPERIMENTAL_ENV_VAR_PREFIX + "PACKAGE_PERFORMANCE"
51+
)
4952
IaCsSupport = {
5053
"terraform": ExperimentalEntry(
5154
"experimentalTerraformSupport", EXPERIMENTAL_ENV_VAR_PREFIX + "TERRAFORM_SUPPORT"

samcli/lib/package/artifact_exporter.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
from botocore.utils import set_value_from_jmespath
2121

22+
from samcli.commands._utils.experimental import ExperimentalFlag, is_experimental_enabled
2223
from samcli.commands.package import exceptions
2324
from samcli.lib.package.code_signer import CodeSigner
2425
from samcli.lib.package.local_files_utils import get_uploaded_s3_object_name, mktempfile
@@ -275,6 +276,10 @@ def export(self) -> Dict:
275276
self._apply_global_values()
276277
self.template_dict = self._export_global_artifacts(self.template_dict)
277278

279+
cache: Optional[Dict] = None
280+
if is_experimental_enabled(ExperimentalFlag.PackagePerformance):
281+
cache = {}
282+
278283
for resource_logical_id, resource in self.template_dict["Resources"].items():
279284
resource_type = resource.get("Type", None)
280285
resource_dict = resource.get("Properties", {})
@@ -287,7 +292,7 @@ def export(self) -> Dict:
287292
if resource_dict.get("PackageType", ZIP) != exporter_class.ARTIFACT_TYPE:
288293
continue
289294
# Export code resources
290-
exporter = exporter_class(self.uploaders, self.code_signer)
295+
exporter = exporter_class(self.uploaders, self.code_signer, cache)
291296
exporter.export(full_path, resource_dict, self.template_dir)
292297

293298
return self.template_dict

samcli/lib/package/packageable_resources.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,10 @@ class Resource:
6767
EXPORT_DESTINATION: Destination
6868
ARTIFACT_TYPE: Optional[str] = None
6969

70-
def __init__(self, uploaders: Uploaders, code_signer):
70+
def __init__(self, uploaders: Uploaders, code_signer, cache: Optional[Dict] = None):
7171
self.uploaders = uploaders
7272
self.code_signer = code_signer
73+
self.cache = cache
7374

7475
@property
7576
def uploader(self) -> Union[S3Uploader, ECRUploader]:
@@ -167,6 +168,7 @@ def do_export(
167168
uploader,
168169
artifact_extension,
169170
local_path,
171+
self.cache,
170172
)
171173
if should_sign_package:
172174
uploaded_url = self.code_signer.sign_package(

samcli/lib/package/utils.py

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ def upload_local_artifacts(
141141
uploader: S3Uploader,
142142
extension: Optional[str] = None,
143143
local_path: Optional[str] = None,
144+
previously_uploaded: Optional[Dict] = None,
144145
) -> str:
145146
"""
146147
Upload local artifacts referenced by the property at given resource and
@@ -154,18 +155,20 @@ def upload_local_artifacts(
154155
155156
If path is already a path to S3 object, this method does nothing.
156157
157-
:param resource_type: Type of the CloudFormation resource
158-
:param resource_id: Id of the CloudFormation resource
159-
:param resource_dict: Dictionary containing resource definition
160-
:param property_path: Json path to the property of SAM or CloudFormation resource where the
161-
local path is present
162-
:param parent_dir: Resolve all relative paths with respect to this
163-
directory
164-
:param uploader: Method to upload files to S3
165-
:param extension: Extension of the uploaded artifact
166-
:param local_path: Local path for the cases when search return more than single result
167-
:return: S3 URL of the uploaded object
168-
:raise: ValueError if path is not a S3 URL or a local path
158+
:param resource_type: Type of the CloudFormation resource
159+
:param resource_id: Id of the CloudFormation resource
160+
:param resource_dict: Dictionary containing resource definition
161+
:param property_path: Json path to the property of SAM or CloudFormation resource
162+
where the local path is present
163+
:param parent_dir: Resolve all relative paths with respect to this
164+
directory
165+
:param uploader: Method to upload files to S3
166+
:param extension: Extension of the uploaded artifact
167+
:param local_path: Local path for the cases when search return more than single result
168+
:param previously_uploaded: Cache mapping from (evaluated) local path to previous result of
169+
this function
170+
:return: S3 URL of the uploaded object
171+
:raise: ValueError if path is not a S3 URL or a local path
169172
"""
170173

171174
if local_path is None:
@@ -183,18 +186,29 @@ def upload_local_artifacts(
183186

184187
local_path = make_abs_path(parent_dir, local_path)
185188

189+
if previously_uploaded and local_path in previously_uploaded:
190+
result = previously_uploaded[local_path]
191+
LOG.debug("Skipping upload of %s since is already uploaded to %s", local_path, result)
192+
return cast(str, result)
193+
186194
# Or, pointing to a folder. Zip the folder and upload (zip_method is changed based on resource type)
187195
if is_local_folder(local_path):
188-
return zip_and_upload(
196+
result = zip_and_upload(
189197
local_path,
190198
uploader,
191199
extension,
192200
zip_method=make_zip_with_lambda_permissions if resource_type in LAMBDA_LOCAL_RESOURCES else make_zip,
193201
)
202+
if previously_uploaded is not None:
203+
previously_uploaded[local_path] = result
204+
return cast(str, result)
194205

195206
# Path could be pointing to a file. Upload the file
196207
if is_local_file(local_path):
197-
return uploader.upload_with_dedup(local_path)
208+
result = uploader.upload_with_dedup(local_path)
209+
if previously_uploaded is not None:
210+
previously_uploaded[local_path] = result
211+
return cast(str, result)
198212

199213
raise InvalidLocalPathError(resource_id=resource_id, property_name=property_path, local_path=local_path)
200214

tests/unit/commands/_utils/test_experimental.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,16 @@ def test_set_experimental(self):
5757
self.gc_mock.return_value.set_value.assert_called_once_with(config_entry, False, is_flag=True, flush=False)
5858

5959
def test_get_all_experimental(self):
60-
self.assertEqual(len(get_all_experimental()), 4)
60+
self.assertEqual(len(get_all_experimental()), 5)
6161

6262
def test_get_all_experimental_statues(self):
63-
self.assertEqual(len(get_all_experimental_statues()), 4)
63+
self.assertEqual(len(get_all_experimental_statues()), 5)
6464

6565
def test_get_all_experimental_env_vars(self):
66-
self.assertEqual(len(get_all_experimental_env_vars()), 4)
66+
self.assertEqual(len(get_all_experimental_env_vars()), 5)
6767

6868
def test_get_enabled_experimental_flags(self):
69-
self.assertEqual(len(get_enabled_experimental_flags()), 4)
69+
self.assertEqual(len(get_enabled_experimental_flags()), 5)
7070

7171
@patch("samcli.commands._utils.experimental.set_experimental")
7272
@patch("samcli.commands._utils.experimental.get_all_experimental")

0 commit comments

Comments
 (0)