aws / aws/sagemaker-python-sdk

ParamValidationError thrown when setting parallelism_config in pipeline.upsert, pipeline.create etc.

Open
#5,354 0 comments 0 reactions 0 assignees View on GitHub
component: pipelines type: bug
Dominant language
Python
Stars
2.3k
Forks
1.3k
Avg merge
1d 22h
Merged PRs (30d)
35

Description

# Quick version

## The Problem
- When calling `pipeline.upsert` ([code](https://github.com/qidewenwhen/sagemaker-python-sdk/blob/57c2b941d7bb712f2fe1870381e5ff2bc809ee66/src/sagemaker/workflow/pipeline.py)), if you pass in a`parallelism_config` value with the correct type of ParallelIismConfiguration like this:
```
pipeline.upsert(
config.sm_role,
parallelism_config=ParallelismConfiguration(max_parallel_execution_steps=5),
)
```
boto3 throws a `ParamValidationError`.
```
ParamValidationError: Parameter validation failed:
Invalid type for parameter ParallelismConfiguration, value: , type: , valid types:
```
- Boto3 is expecting a dict, and isn't able to handle the ParallelismConfiguration object, which is passed through to it.
- If you convert the ParallelismConfiguration to a dict before passing it in, using the `to_request()` method, the call succeeds.
- However, this does require passing in an object with an incorrect type, which angers the type checker. A recent commit has enabled type checking for this module, which caused this problem to show up.
- The tests in `test_workflow.py` [erroneously pass in a dict](https://github.com/qidewenwhen/sagemaker-python-sdk/blob/57c2b941d7bb712f2fe1870381e5ff2bc809ee66/tests/integ/sagemaker/workflow/test_workflow.py#L1006), rather than the correct ParallelismConfiguration object.
- Discovered in `v2.254.1`. It's been present for...quite a while I think. Issue became obvious in v2.245.0 when type validation was enabled.
- I quickly checked pipeline.create and pipeline.update too - the behaviour seems to be the same.

## The fix
Presumably, call `to_request()` somewhere in workflows/pipeline.py when handling the ParallelConfiguration parameters.

# Long Version
- We recently upgraded from v2.243.2 to v2.254.1
- This included the [fix that enabled type checking](https://github.com/aws/sagemaker-python-sdk/commit/5ce7249ee6c11033bbe4ab9e6f8879198245762e#diff-22d6ae36367d72d3712cabce03e290ce3ceaded610d5dd4eaffb59619836e773):
- Our build failed our mypy type checking, as we were calling pipeline.upsert like this:

```
pipeline.upsert(
config.sm_role,
parallelism_config=ParallelismConfiguration(max_parallel_execution_steps=5).to_request(),
)
```
`to_request()` was converting the ParallelismConfiguration into a RequestType, which under the hood was a dict.
- The definition for upsert has long been
```
def upsert(
self,
role_arn: str = None,
description: str = None,
tags: Optional[Tags] = None,
parallelism_config: ParallelismConfiguration = None,
) -> Dict[str, Any]:
```
- The fix of the types therefore caused our type validator mypy to flag the issue - we were passing in a dict, and the correct type was a ParallelismConfiguration.
- However, when we fixed it, removing the `to_request()` call, we get the following error when invoking the method:
```
ParamValidationError: Parameter validation failed:
Invalid type for parameter ParallelismConfiguration, value: , type: , valid types:
```

Contributor guide

Open the contributing guide

Research direction

Start in src/sagemaker/workflow/pipeline.py and trace how parallelism_config is passed by pipeline.upsert, pipeline.create, and pipeline.update. Review tests/integ/sagemaker/workflow/test_workflow.py, especially the parallelism configuration case, and run the relevant workflow tests. Done means these methods accept ParallelismConfiguration objects and boto3 no longer raises ParamValidationError.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, machine-learning
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.