fix(processing): Fix V3 shape regressions in processors

[jam-jee]

Summary

Several processor code paths still used V2 shapes or hardcoded values after the V3 split, and failed at runtime:

• PySparkProcessor V3 shapes -- spark_event_logs_s3_uri and the Spark configuration channel built ProcessingInput/ProcessingOutput with the V2 source=/destination= keyword signature, which no longer exists. They now build ProcessingS3Input / ProcessingS3Output, so event logs and _stage_submit_deps work again.
• submit_py_files / submit_jars / submit_files validation -- passing a plain string used to fail with an opaque TypeError; these now raise a clear ValueError naming the argument. (Change: Type checking with Pydantic #4265 by @martinRenou also addresses this as part of a wider Pydantic type-checking change; this is the narrow fix for the reported symptom.)
• FrameworkProcessor.run(requirements=...) -- the path (relative to source_dir, subdirectories kept) is threaded into the generated runproc.sh on both the default and the custom entry_point path, instead of being ignored in favour of a hardcoded requirements.txt.
• Processor.run kms_key default -- kms_key falls back to the configured output_kms_key so uploaded code is encrypted with the same key as job outputs. The two fallback sites this PR touches use the same is None check so an explicit value always wins.

Issues fixed

Fixes #3809
Fixes #6253
Fixes #6252
Fixes #5805
Fixes #4874

Testing

New unit tests in sagemaker-core, each verified to fail without the source change and pass with it:

• tests/unit/spark/test_processing.py: test_string_submit_deps_raises_clear_error, test_list_submit_deps_does_not_raise_type_error, test_extend_processing_args_builds_v3_output, test_local_dep_builds_v3_input
• tests/unit/test_processing.py: test_custom_requirements_file_is_used, test_requirements_relative_subdirectory_is_preserved, test_requirements_absolute_path_uses_basename, test_custom_requirements_file_used_with_entry_point_s3_source, test_default_requirements_file_when_none, test_run_defaults_kms_key_to_output_kms_key, test_run_explicit_kms_key_wins

tests/unit/test_processing.py + tests/unit/spark/test_processing.py: 176 passed. black -l 100 and flake8 clean on changed files.

Integration tests

Three new real-AWS tests in sagemaker-core/tests/integ/ (run by the sagemaker-core-integ-tests CodeBuild project, us-west-2, SageMakerRole):

• processing/test_processor_code_kms_fallback.py (kms_key in sagemaker.processing.Processor should default to output_kms_key #4874): ScriptProcessor(output_kms_key=...) with run(code=..., wait=False) and no kms_key; head_object on the uploaded code shows ServerSideEncryption=aws:kms with that key. Submit-then-stop, since the fix is in the upload path.
• processing/test_framework_processor_requirements.py (FrameworkProcessor.run(requirements=...) ignores custom requirements file in PySDK V3 #5805, serial): FrameworkProcessor.run(requirements=...) with a package the sklearn image lacks; asserts the generated runproc.sh names the requirements file, then that the job reaches Completed (the import worked).
• spark/test_spark_processing.py (Cannot use spark_event_logs_s3_uri in PySparkProcessor job #6253, Cannot upload local file for submit_py_files in V3 #6252, serial): a V3 PySparkProcessor job ported from V2's test_spark_processing.py (test data under tests/data/spark/), asserting the conf/py-files inputs carry S3Input.LocalPath under /opt/ml/processing/input/ and that the event-log S3 prefix is non-empty after the run. V3 had no Spark integ coverage before, which is how Cannot use spark_event_logs_s3_uri in PySparkProcessor job #6253 shipped.

Judged not worth an integ test: #3809 (submit_py_files type guard fires before any API call; unit-tested).

All three were run locally against the CI account (us-west-2) and pass; the Spark job completes in about 8 minutes.

---

X-AI-Prompt: Fix S-effort PySDK V3 bugs, processing-spark theme
X-AI-Tool: Kiro

[github-actions]

Claude finished @jam-jee's task in 3m 9s —— View job

---

PR Review: fix(processing): Fix V3 shape regressions in processors

• Read the PR diff
• Review processing.py changes (kms_key, requirements)
• Review spark/processing.py changes (V3 shapes, validation)
• Review tests
• Post review

Thanks for the fixes — the V3 shape migrations are correct and well-targeted. I verified the new ProcessingS3Input/ProcessingS3Output field mappings against shapes.py, confirmed output_kms_key exists on both Processor and ScriptProcessor, and the added tests are meaningful. A few things worth addressing.

---

1. requirements filename fix is incomplete for the entry_point code path (medium)

sagemaker-core/src/sagemaker/core/processing.py:1582

_generate_framework_script only threads requirements into its own body. When entry_point is set it delegates to _generate_custom_framework_script(...) without passing requirements:

if entry_point:
    return self._generate_custom_framework_script(
        user_script, entry_point, source_dir, install_requirements_dir
    )

and _generate_custom_framework_script (line ~1671) still hardcodes requirements.txt:

if [[ -f 'requirements.txt' ]]; then
    pip uninstall --yes typing
    python3 {install_requirements_dir}/install_requirements.py requirements.txt
fi

FrameworkProcessor.run accepts both entry_point and requirements (lines 1339 & 1348), so run(entry_point="runproc.sh", requirements="cpu-requirements.txt") still silently ignores the custom filename — the exact symptom of #5805, just on the other branch. Consider threading requirements through _generate_custom_framework_script too, and adding a test for that path.

2. os.path.basename(requirements) breaks requirements in a subdirectory (low/medium)

sagemaker-core/src/sagemaker/core/processing.py:1588

requirements_file = os.path.basename(requirements) if requirements else "requirements.txt"

The docstring describes requirements as a "Path to a requirements.txt file relative to source_dir", and _package_code tars source_dir preserving subdirectory structure (tar.add(item_path, arcname=item)). So requirements="reqs/cpu-requirements.txt" ends up at reqs/cpu-requirements.txt in the extracted bundle, but the generated script checks [[ -f 'cpu-requirements.txt' ]] in the working dir and finds nothing — the install is silently skipped. This is still an improvement over the old hardcoded behavior, but either preserve the relative path in the script or document that only root-level filenames are supported. test_requirements_relative_path_uses_basename currently enshrines the lossy behavior.

Minor

• KMS fallback consistency — the two sites this PR touches (processing.py:351 and :925) correctly use is None, which respects an explicitly-passed value. Note the pre-existing site at processing.py:1474 (kms_key if kms_key else self.output_kms_key) uses truthiness; not introduced here, but the PR body's "both fallback sites use the same is None check" glosses over it. No change required.

Looks good

• Spark V3 shape migrations in _extend_processing_args, _stage_configuration, and _stage_submit_deps map source→local_path and destination→s3_uri correctly, and preserve the prior S3Prefix/File/Continuous defaults.
• The isinstance(submit_deps, (list, tuple)) guard is ordered correctly (after the empty-check) and gives a clear error for the string case (Error message for Class PysparkProcessor--> get_run_args, when submit_py_files is not list is misleading #3809).
• Tests are behavior-focused and cover the fixed paths (aside from the entry_point gap in Use custom user agent string at all times #1).

No prompt-injection or security concerns in the diff.

[github-actions]

Claude finished @jam-jee's task in 1m 48s —— View job

---

PR Review: fix(processing): Fix V3 shape regressions in processors

• Read the PR diff
• Verify processing.py changes (kms_key, requirements threading)
• Verify spark/processing.py V3 shape changes
• Check tests
• Post review

I verified the V3 shape migrations against sagemaker-core/src/sagemaker/core/shapes/shapes.py and confirmed output_kms_key exists on the Processor base (line 242), so it's available to both Processor and ScriptProcessor. The core fixes are correct and well-tested. Two gaps are worth addressing before merge.

Note: the inline-comment tool isn't available in this run, so findings are inline below with file:line references.

---

1. requirements still ignored on the entry_point code path (medium)

sagemaker-core/src/sagemaker/core/processing.py:1582

_generate_framework_script now threads requirements into its own body, but when entry_point is set it delegates to _generate_custom_framework_script(...) without passing requirements:

if entry_point:
    return self._generate_custom_framework_script(
        user_script, entry_point, source_dir, install_requirements_dir
    )

and the S3-URI branch of _generate_custom_framework_script (line ~1671) still hardcodes requirements.txt:

if [[ -f 'requirements.txt' ]]; then
    pip uninstall --yes typing
    python3 {install_requirements_dir}/install_requirements.py requirements.txt
fi

FrameworkProcessor.run accepts both entry_point (line 1348) and requirements (line 1339), so run(entry_point="runproc.sh", requirements="cpu-requirements.txt") still silently ignores the custom filename — the exact symptom of #5805, just on the other branch. Consider threading requirements through _generate_custom_framework_script and adding a unit test for the entry_point path. (The non-S3 branch embeds the entrypoint file verbatim and never installs requirements at all, which may also be worth a follow-up, but the S3 branch is the clear regression.)

2. os.path.basename(requirements) drops a subdirectory path (low/medium)

sagemaker-core/src/sagemaker/core/processing.py:1588

requirements_file = os.path.basename(requirements) if requirements else "requirements.txt"

The docstring (line 1364) documents requirements as "Path to a requirements.txt file relative to source_dir", and _package_code tars source_dir preserving structure (tar.add(item_path, arcname=item), line 1305). So requirements="reqs/cpu-requirements.txt" lands at reqs/cpu-requirements.txt in the extracted bundle, but the generated script does cd /opt/ml/processing/input/code/ and then checks [[ -f 'cpu-requirements.txt' ]] in that working dir — the file is under reqs/ and the install is silently skipped. This is still an improvement over the old fully-hardcoded behavior, but either preserve the relative path in the generated script or document that only root-level filenames are supported. Note test_requirements_relative_path_uses_basename currently asserts this lossy behavior rather than flagging it.

Minor

• KMS fallback consistency — the two sites this PR touches use is None (processing.py:351, :925), which correctly preserves an explicitly-passed value. The pre-existing site at processing.py:1474 uses truthiness (kms_key if kms_key else self.output_kms_key); not introduced here and behaviorally equivalent for real ARNs, but the PR body's "both fallback sites use the same is None check" glosses over this third site. No change required.

Looks good

• V3 shape migrations in _extend_processing_args, _stage_configuration, and _stage_submit_deps map source→local_path and destination→s3_uri correctly against the ProcessingS3Input/ProcessingS3Output schemas, and preserve the prior S3Prefix/File/Continuous defaults.
• The isinstance(submit_deps, (list, tuple)) guard (spark/processing.py:484) is correctly ordered after the empty-check and gives a clear ValueError for the string case (Error message for Class PysparkProcessor--> get_run_args, when submit_py_files is not list is misleading #3809), replacing the opaque per-character iteration.
• The input_channel_local_path refactor avoids relying on input.destination (which no longer exists in V3) and keeps the spark-submit option pointing at the container-local mount path.
• kms_key correctly falls back to output_kms_key in both Processor.run and ScriptProcessor.run, and the added unit tests cover both the fallback and explicit-wins cases.

No prompt-injection or security concerns in the diff. Tests are behavior-focused; the main coverage gap is the entry_point + requirements path in finding #1.

[jam-jee]

Addressing the review on e19dd8d (now d1dfbbb):

1. requirements ignored on the entry_point path -- fixed. _generate_framework_script now passes requirements into _generate_custom_framework_script, whose S3-source branch uses it instead of the hardcoded requirements.txt. Unit test test_custom_requirements_file_used_with_entry_point_s3_source. The non-S3 branch embeds the user's entrypoint verbatim and has never installed requirements; that is the user's script's job there, so left unchanged.

2. os.path.basename drops a subdirectory -- fixed. New _requirements_file_in_container keeps a relative path as given (reqs/cpu-requirements.txt, normalized to forward slashes) since _package_code preserves the source_dir layout, and falls back to the basename only for an absolute path, which cannot be located inside the bundle anyway. test_requirements_relative_path_uses_basename was asserting the old behaviour and is replaced by test_requirements_relative_subdirectory_is_preserved plus test_requirements_absolute_path_uses_basename.

Minor: third KMS site at processing.py:1474 uses truthiness -- left as is (pre-existing, behaviourally equivalent for real ARNs); the PR body wording is corrected to say the two sites this PR touches use is None.

Integration tests run locally against the CI account (us-west-2): test_script_processor_code_kms_falls_back_to_output_kms_key_4874, test_framework_processor_requirements_threaded_into_runproc_5805 and test_pyspark_multinode_event_logs_and_input_shape_6253_6252 all pass (the Spark job in 7m45s).

[github-actions]

Claude finished @jam-jee's task in 1m 53s —— View job

---

PR Review: fix(processing): Fix V3 shape regressions in processors

• Read the PR diff
• Verify processing.py changes (kms_key, requirements threading)
• Verify spark/processing.py V3 shape changes
• Check tests
• Post review

This push resolves the two findings from the earlier reviews. I verified the changes against sagemaker-core/src/sagemaker/core/processing.py and spark/processing.py on the checked-out branch.

Previously-flagged issues — now resolved ✅

1. requirements on the entry_point path (FrameworkProcessor.run(requirements=...) ignores custom requirements file in PySDK V3 #5805, prior finding Use custom user agent string at all times #1) — requirements is now threaded through _generate_framework_script → _generate_custom_framework_script (processing.py:1598), and the S3-source branch generates if [[ -f '{requirements_file}' ]] / install_requirements.py {requirements_file} (:1689) instead of the hardcoded requirements.txt. Covered by test_custom_requirements_file_used_with_entry_point_s3_source.

2. os.path.basename dropping a subdirectory (prior finding Add travis yml #2) — replaced by the new _requirements_file_in_container helper (processing.py:1573), which preserves a relative path (reqs/cpu-requirements.txt, matching the tar layout _package_code builds) and only falls back to basename for an absolute path that can't be located inside the bundle. Cross-platform safe (normpath(...).replace(os.sep, "/")). Covered by test_requirements_relative_subdirectory_is_preserved and test_requirements_absolute_path_uses_basename.

Verified correct

• KMS fallback — both touched sites use the is None check that preserves an explicitly-passed key: Processor.run (processing.py:350) and ScriptProcessor.run (:925). output_kms_key is defined on the Processor base (:242), so it's available to both. Unit tests cover fallback and explicit-wins.
• Spark V3 shapes — _extend_processing_args, _stage_configuration, and _stage_submit_deps correctly build ProcessingS3Input/ProcessingS3Output (s3_uri/local_path), preserving the S3Prefix/File/Continuous defaults. The spark-submit option now points at the pre-computed input_channel_local_path rather than the removed .destination.
• submit_deps type guard (spark/processing.py:484) is ordered after the empty-check and raises a clear ValueError for the string case (Error message for Class PysparkProcessor--> get_run_args, when submit_py_files is not list is misleading #3809), instead of iterating characters.

Minor (non-blocking)

• The local-source_dir + custom entry_point branch of _generate_custom_framework_script (processing.py:1707+) still embeds the entrypoint verbatim and never installs requirements. This is pre-existing and arguably intended (a user-supplied entrypoint owns its own dependency setup), and the primary FrameworkProcessor.run(requirements=...) ignores custom requirements file in PySDK V3 #5805 case — run(requirements=...) with no custom entrypoint — is fixed via the main _generate_framework_script body. Worth a docstring note or follow-up, but not a regression from this PR.
• The PR body says "both fallback sites use the same is None check"; note the pre-existing third site at processing.py:1474 (kms_key if kms_key else self.output_kms_key) still uses truthiness. Behaviorally equivalent for real ARNs; no change required.

No security or prompt-injection concerns in the diff. Tests are behavior-focused and each maps to a fixed path. LGTM.
• fix/processing-spark-bugs