Skip to content

Commit b5a4d37

Browse files
authored
fix: use pipeline code_hash for FrameworkProcessor sourcedir upload (#6494)
* fix: use pipeline code_hash for FrameworkProcessor sourcedir upload FrameworkProcessor._package_code() now uses _pipeline_config.code_hash in the S3 key when available, matching the behavior of ScriptProcessor._upload_code(). This enables pipeline cache hits for FrameworkProcessor jobs with source_dir. Fixes #6493 * fix: keep sourcedir.tar.gz under pipeline code_hash prefix Preserve the sourcedir.tar.gz filename under the pipeline code-hash prefix so _pack_and_upload_code helper replace() URIs stay distinct. Add a regression test covering _pack_and_upload_code with two hashes.
1 parent 9aeb4b4 commit b5a4d37

2 files changed

Lines changed: 122 additions & 6 deletions

File tree

‎sagemaker-core/src/sagemaker/core/processing.py‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,12 +1336,24 @@ def _package_code(
13361336
raise ValueError(f"Dependency path does not exist: {dep_path}")
13371337
tar.add(dep_path, arcname=os.path.basename(dep_path))
13381338

1339-
s3_uri = s3.s3_path_join(
1340-
self._s3_code_prefix(),
1341-
job_name,
1342-
"source",
1343-
"sourcedir.tar.gz",
1344-
)
1339+
# Use pipeline code_hash for content-addressed caching when available
1340+
from sagemaker.core.workflow.utilities import _pipeline_config
1341+
1342+
if _pipeline_config and _pipeline_config.code_hash:
1343+
s3_uri = s3.s3_path_join(
1344+
self._s3_code_prefix(),
1345+
_pipeline_config.pipeline_name,
1346+
"source",
1347+
_pipeline_config.code_hash,
1348+
"sourcedir.tar.gz",
1349+
)
1350+
else:
1351+
s3_uri = s3.s3_path_join(
1352+
self._s3_code_prefix(),
1353+
job_name,
1354+
"source",
1355+
"sourcedir.tar.gz",
1356+
)
13451357

13461358
with open(tmp.name, "rb") as tar_file:
13471359
tar_bytes = tar_file.read()

‎sagemaker-core/tests/unit/test_processing.py‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,6 +1411,45 @@ def checking_unlink(path, *args, **kwargs):
14111411
observed.get("still_open") == []
14121412
), "temp tar.gz handle was still open when os.unlink was called"
14131413

1414+
def test_package_code_with_pipeline_config(self, mock_session):
1415+
"""Test that _package_code uses pipeline code_hash when available.
1416+
1417+
When _pipeline_config.code_hash is set, _package_code should use it
1418+
in the S3 key (similar to ScriptProcessor._upload_code) for pipeline
1419+
cache hits. This test verifies the fix for issue #6493.
1420+
"""
1421+
processor = FrameworkProcessor(
1422+
role="arn:aws:iam::123456789012:role/SageMakerRole",
1423+
image_uri="test-image:latest",
1424+
instance_count=1,
1425+
instance_type="ml.m5.xlarge",
1426+
sagemaker_session=mock_session,
1427+
)
1428+
1429+
with tempfile.TemporaryDirectory() as tmpdir:
1430+
entry_point = os.path.join(tmpdir, "train.py")
1431+
with open(entry_point, "w") as f:
1432+
f.write("print('training')")
1433+
1434+
with patch("sagemaker.core.workflow.utilities._pipeline_config") as mock_config:
1435+
mock_config.pipeline_name = "test-pipeline"
1436+
mock_config.code_hash = "abc123"
1437+
1438+
result = processor._package_code(
1439+
entry_point=entry_point,
1440+
source_dir=tmpdir,
1441+
requirements=None,
1442+
job_name="test-job",
1443+
kms_key=None,
1444+
)
1445+
1446+
# Should use code_hash in the S3 path, not job_name, and keep
1447+
# sourcedir.tar.gz so helper-script replace() URIs stay distinct.
1448+
assert "test-pipeline" in result
1449+
assert "abc123" in result
1450+
assert "sourcedir.tar.gz" in result
1451+
assert "test-job" not in result
1452+
14141453

14151454
class TestFrameworkProcessorRun:
14161455
def test_run_with_s3_code(self, mock_session):
@@ -1631,6 +1670,71 @@ def test_pack_and_upload_code_explicit_kms_key_wins(self, mock_session):
16311670
upload_kms = [call.kwargs.get("kms_key") for call in mock_upload.call_args_list]
16321671
assert upload_kms and all(k == explicit_kms for k in upload_kms)
16331672

1673+
def test_pack_and_upload_code_with_pipeline_config(self, mock_session):
1674+
"""Pipeline code_hash prefix must keep sourcedir.tar.gz so helper uploads do not collide.
1675+
1676+
``_pack_and_upload_code`` derives runproc.sh and install_requirements.py URIs
1677+
with ``s3_payload.replace("sourcedir.tar.gz", ...)``. If the payload URI is
1678+
only the hash (no sourcedir.tar.gz filename), those replacements are no-ops
1679+
and the helper scripts overwrite the source archive. Two different hashes
1680+
must also produce different S3 keys (issue #6493).
1681+
"""
1682+
processor = FrameworkProcessor(
1683+
role="arn:aws:iam::123456789012:role/SageMakerRole",
1684+
image_uri="test-image:latest",
1685+
instance_count=1,
1686+
instance_type="ml.m5.xlarge",
1687+
sagemaker_session=mock_session,
1688+
)
1689+
1690+
def pack_with_hash(code_hash):
1691+
with tempfile.TemporaryDirectory() as tmpdir:
1692+
entry_point = os.path.join(tmpdir, "train.py")
1693+
with open(entry_point, "w") as f:
1694+
f.write("print('training')")
1695+
1696+
with patch("sagemaker.core.workflow.utilities._pipeline_config") as mock_config:
1697+
mock_config.pipeline_name = "test-pipeline"
1698+
mock_config.code_hash = code_hash
1699+
with patch(
1700+
"sagemaker.core.s3.S3Uploader.upload_string_as_file_body"
1701+
) as mock_upload:
1702+
mock_upload.side_effect = (
1703+
lambda body, desired_s3_uri=None, **kwargs: desired_s3_uri
1704+
)
1705+
processor._pack_and_upload_code(
1706+
code=entry_point,
1707+
source_dir=tmpdir,
1708+
requirements=None,
1709+
job_name="test-job",
1710+
inputs=None,
1711+
kms_key=None,
1712+
)
1713+
upload_uris = [
1714+
call.kwargs.get("desired_s3_uri") for call in mock_upload.call_args_list
1715+
]
1716+
1717+
sourcedir_uris = [uri for uri in upload_uris if uri and "sourcedir.tar.gz" in uri]
1718+
helper_uris = [
1719+
uri
1720+
for uri in upload_uris
1721+
if uri and ("install_requirements.py" in uri or uri.endswith("/runproc.sh"))
1722+
]
1723+
assert sourcedir_uris, "sourcedir.tar.gz must remain in the pipeline hash S3 key"
1724+
payload_uri = sourcedir_uris[0]
1725+
assert code_hash in payload_uri
1726+
assert "test-pipeline" in payload_uri
1727+
assert "test-job" not in payload_uri
1728+
assert helper_uris, "expected helper-script uploads"
1729+
assert payload_uri not in helper_uris
1730+
return payload_uri
1731+
1732+
uri_a = pack_with_hash("abc123")
1733+
uri_b = pack_with_hash("def456")
1734+
assert uri_a != uri_b
1735+
assert "abc123" in uri_a
1736+
assert "def456" in uri_b
1737+
16341738

16351739
class TestProcessingInputOutputHelpers:
16361740
def test_processing_input_with_app_managed(self):

0 commit comments

Comments
 (0)