Skip to content

Commit 193df0f

Browse files
rsareddy0329Roja Reddy Sareddy
andauthored
fix(processing): respect session region when waiting for processing jobs (#6369)
Processor.run / FrameworkProcessor.run waited via self.latest_job.wait(), whose generated refresh() resolves a default, global SageMaker client (Base.get_sagemaker_client()) rather than the region of the sagemaker_session that created the job. With a non-default-region session the wait/describe and log tailing hit the wrong region and fail (issue #5796). Route the wait path through the session instead: - logs=True -> logs_for_processing_job(sagemaker_session, ...), which already resolves clients from sagemaker_session; also fix it to describe via sagemaker_session.sagemaker_client (region-aware) instead of ProcessingJob.get(...).refresh() (the chained refresh used the global client and read PascalCase keys off a snake_case resource __dict__). - logs=False -> new _wait_for_processing_job(sagemaker_session, job_name), which polls describe_processing_job through the session's region-aware client and raises on failure. Add unit tests covering both wait paths and _wait_for_processing_job. Relates-to #5796 Co-authored-by: Roja Reddy Sareddy <rsareddy@amazon.com>
1 parent 241bd62 commit 193df0f

2 files changed

Lines changed: 127 additions & 15 deletions

File tree

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

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,17 @@ def run(
361361
if not isinstance(self.sagemaker_session, PipelineSession):
362362
self.jobs.append(self.latest_job)
363363
if wait:
364-
self.latest_job.wait(logs=logs)
364+
if logs:
365+
logs_for_processing_job(
366+
sagemaker_session=self.sagemaker_session,
367+
job_name=self.latest_job.processing_job_name,
368+
wait=True,
369+
)
370+
else:
371+
_wait_for_processing_job(
372+
sagemaker_session=self.sagemaker_session,
373+
job_name=self.latest_job.processing_job_name,
374+
)
365375

366376
def _extend_processing_args(self, inputs, outputs, **kwargs): # pylint: disable=W0613
367377
"""Extend inputs and outputs based on extra parameters"""
@@ -929,7 +939,17 @@ def run(
929939
if not isinstance(self.sagemaker_session, PipelineSession):
930940
self.jobs.append(self.latest_job)
931941
if wait:
932-
self.latest_job.wait(logs=logs)
942+
if logs:
943+
logs_for_processing_job(
944+
sagemaker_session=self.sagemaker_session,
945+
job_name=self.latest_job.processing_job_name,
946+
wait=True,
947+
)
948+
else:
949+
_wait_for_processing_job(
950+
sagemaker_session=self.sagemaker_session,
951+
job_name=self.latest_job.processing_job_name,
952+
)
933953

934954
def _include_code_in_inputs(self, inputs, code, kms_key=None):
935955
"""Converts code to appropriate input and includes in input list.
@@ -1832,6 +1852,36 @@ def _get_process_request(
18321852
return process_request
18331853

18341854

1855+
def _wait_for_processing_job(sagemaker_session, job_name, poll=10):
1856+
"""Wait for a processing job to reach a terminal state, respecting the session region.
1857+
1858+
Unlike ``ProcessingJob.wait()`` (which resolves a default, global SageMaker client),
1859+
this describes the job through ``sagemaker_session.sagemaker_client`` so the job is
1860+
polled in the same region it was created in (issue #5796).
1861+
1862+
Args:
1863+
sagemaker_session: The session used to create the job; its region-aware
1864+
``sagemaker_client`` is used to describe the job.
1865+
job_name (str): Name of the processing job to wait for.
1866+
poll (int): The interval in seconds between polling for job completion.
1867+
1868+
Raises:
1869+
ValueError: If the processing job fails.
1870+
"""
1871+
terminal_states = ("Completed", "Failed", "Stopped")
1872+
1873+
def _describe_if_terminal():
1874+
description = sagemaker_session.sagemaker_client.describe_processing_job(
1875+
ProcessingJobName=job_name
1876+
)
1877+
if description["ProcessingJobStatus"] in terminal_states:
1878+
return description
1879+
return None
1880+
1881+
description = _wait_until(_describe_if_terminal, poll)
1882+
_check_job_status(job_name, description, "ProcessingJobStatus")
1883+
1884+
18351885
def logs_for_processing_job(sagemaker_session, job_name, wait=False, poll=10):
18361886
"""Display logs for a given processing job, optionally tailing them until the is complete.
18371887
@@ -1847,11 +1897,9 @@ def logs_for_processing_job(sagemaker_session, job_name, wait=False, poll=10):
18471897
"""
18481898

18491899
description = _wait_until(
1850-
lambda: ProcessingJob.get(
1851-
processing_job_name=job_name, session=sagemaker_session.boto_session
1852-
)
1853-
.refresh()
1854-
.__dict__,
1900+
lambda: sagemaker_session.sagemaker_client.describe_processing_job(
1901+
ProcessingJobName=job_name
1902+
),
18551903
poll,
18561904
)
18571905

@@ -1901,12 +1949,8 @@ def logs_for_processing_job(sagemaker_session, job_name, wait=False, poll=10):
19011949
if state == LogState.JOB_COMPLETE:
19021950
state = LogState.COMPLETE
19031951
elif time.time() - last_describe_job_call >= 30:
1904-
description = (
1905-
ProcessingJob.get(
1906-
processing_job_name=job_name, session=sagemaker_session.boto_session
1907-
)
1908-
.refresh()
1909-
.__dict__
1952+
description = sagemaker_session.sagemaker_client.describe_processing_job(
1953+
ProcessingJobName=job_name
19101954
)
19111955
last_describe_job_call = time.time()
19121956

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

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
_processing_output_to_request_dict,
2424
_get_process_request,
2525
logs_for_processing_job,
26+
_wait_for_processing_job,
2627
)
2728
from sagemaker.core.shapes import (
2829
ProcessingInput,
@@ -1099,6 +1100,7 @@ def test_run_with_wait(self, mock_session):
10991100

11001101
mock_job = Mock()
11011102
mock_job.wait = Mock()
1103+
mock_job.processing_job_name = "test-processing-job"
11021104

11031105
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".py") as f:
11041106
f.write("print('test')")
@@ -1110,12 +1112,78 @@ def test_run_with_wait(self, mock_session):
11101112
with patch(
11111113
"sagemaker.core.s3.S3Uploader.upload", return_value="s3://bucket/code.py"
11121114
):
1113-
processor.run(code=temp_file, wait=True, logs=False)
1114-
mock_job.wait.assert_called_once()
1115+
with patch(
1116+
"sagemaker.core.processing._wait_for_processing_job"
1117+
) as mock_wait:
1118+
processor.run(code=temp_file, wait=True, logs=False)
1119+
# The wait path must go through the session-aware helper
1120+
# (which uses the session's region), not latest_job.wait()
1121+
# which resolves a default-region client (issue #5796).
1122+
mock_job.wait.assert_not_called()
1123+
mock_wait.assert_called_once_with(
1124+
sagemaker_session=mock_session,
1125+
job_name="test-processing-job",
1126+
)
11151127
finally:
11161128
if os.path.exists(temp_file):
11171129
os.unlink(temp_file)
11181130

1131+
def test_run_with_wait_and_logs_uses_session(self, mock_session):
1132+
processor = ScriptProcessor(
1133+
role="arn:aws:iam::123456789012:role/SageMakerRole",
1134+
image_uri="test-image:latest",
1135+
command=["python3"],
1136+
instance_count=1,
1137+
instance_type="ml.m5.xlarge",
1138+
sagemaker_session=mock_session,
1139+
)
1140+
1141+
mock_job = Mock()
1142+
mock_job.processing_job_name = "test-processing-job"
1143+
1144+
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".py") as f:
1145+
f.write("print('test')")
1146+
temp_file = f.name
1147+
1148+
try:
1149+
with patch.object(processor, "_start_new", return_value=mock_job):
1150+
with patch("os.path.isfile", return_value=True):
1151+
with patch(
1152+
"sagemaker.core.s3.S3Uploader.upload", return_value="s3://bucket/code.py"
1153+
):
1154+
with patch(
1155+
"sagemaker.core.processing.logs_for_processing_job"
1156+
) as mock_logs:
1157+
processor.run(code=temp_file, wait=True, logs=True)
1158+
# logs path must also be session-aware (issue #5796).
1159+
mock_logs.assert_called_once_with(
1160+
sagemaker_session=mock_session,
1161+
job_name="test-processing-job",
1162+
wait=True,
1163+
)
1164+
finally:
1165+
if os.path.exists(temp_file):
1166+
os.unlink(temp_file)
1167+
1168+
def test_wait_for_processing_job_uses_session_region(self, mock_session):
1169+
"""_wait_for_processing_job describes via the session's region-aware client."""
1170+
mock_session.sagemaker_client.describe_processing_job = Mock(
1171+
return_value={"ProcessingJobStatus": "Completed"}
1172+
)
1173+
with patch("sagemaker.core.processing._wait_until", side_effect=lambda fn, poll: fn()):
1174+
_wait_for_processing_job(mock_session, "test-job", poll=1)
1175+
mock_session.sagemaker_client.describe_processing_job.assert_called_with(
1176+
ProcessingJobName="test-job"
1177+
)
1178+
1179+
def test_wait_for_processing_job_raises_on_failure(self, mock_session):
1180+
mock_session.sagemaker_client.describe_processing_job = Mock(
1181+
return_value={"ProcessingJobStatus": "Failed", "FailureReason": "boom"}
1182+
)
1183+
with patch("sagemaker.core.processing._wait_until", side_effect=lambda fn, poll: fn()):
1184+
with pytest.raises(Exception):
1185+
_wait_for_processing_job(mock_session, "test-job", poll=1)
1186+
11191187
def test_run_without_wait(self, mock_session):
11201188
processor = ScriptProcessor(
11211189
role="arn:aws:iam::123456789012:role/SageMakerRole",

0 commit comments

Comments
 (0)