aws / aws/sagemaker-python-sdk
Setting AsyncPredictor.deserializer doesn't work
- Dominant language
- Python
- Stars
- 2.3k
- Forks
- 1.3k
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 35
Description
**Describe the bug**
An `AsyncPredictor` (returned when deploying an async inference endpoint) exposes `.serializer` and `.deserializer` properties like a typical `Predictor` does... But behaviour of these properties is not consistent in async because of the way the class wraps around an internal `.predictor`.
Overriding a predictor's `.serializer` and `.deserializer` properties after creation is useful and expected functionality (IMO) because:
1. As far as I understand, the current de/serializer class architecture requires setting content_type/accept independently of individual requests. Therefore the only way to switch sent or received content type is by updating the de/serializer or re-creating the entire predictor.
2. In the past, not all predictor classes have supported `serializer` and `deserializer` constructor arguments (see #1997): So some samples have expected to set these properties after constructing.
**To reproduce**
```python
# Create some async predictor:
predictor = some_pytorch_model.deploy(
async_inference_config=sagemaker.async_inference.AsyncInferenceConfig(
output_path="s3://doc-example-bucket/folder",
max_concurrent_invocations_per_instance=2,
),
)
# Override de/serializers (PyTorch defaults to Numpy):
predictor.serializer = sagemaker.serializers.JsonSerializer()
predictor.deserializer = sagemaker.deserializers.JsonDeserializer()
# Make a prediction:
resp = predictor.predict({ "hi": "there" })
```
**Expected behavior**
The endpoint receives a request with `ContentType` and `Accept` matching the configured serializers (`application/json` in the above example).
**Actual behavior**
Because AsyncPredictor [uses its own serializer](https://github.com/aws/sagemaker-python-sdk/blob/5b8eb10bfddcb54fb65c8b28f923697619672120/src/sagemaker/predictor_async.py#L169), the input request is as expected.
...But because it [uses .predictor property's deserializer](https://github.com/aws/sagemaker-python-sdk/blob/5b8eb10bfddcb54fb65c8b28f923697619672120/src/sagemaker/predictor_async.py#L190), the overrides do not affect the response: default NumpyDeserializer and `application/x-npy` Accept headers are still used.
**Screenshots or logs**
N/A
**System information**
A description of your system. Please provide:
- **SageMaker Python SDK version**: 2.86.2 (but checked problem seems to still affect `master` - see links)
- **Framework name (eg. PyTorch) or algorithm (eg. KMeans)**: PyTorch (but should be general)
- **Framework version**: 1.10
- **Python version**: py3.8
- **CPU or GPU**: CPU
- **Custom Docker image (Y/N)**: N
**Additional context**
Today we can work around this by overriding de/serializers on both the outer (async) and inner (sync) predictor objects, as follows:
```python
async_predictor.serializer = JsonSerializer()
async_predictor.deserializer = JsonDeserializer()
async_predictor.predictor.serializer = async_predictor.serializer
async_predictor.predictor.deserializer = async_predictor.deserializer
```
...But I'd recommend a better solution would be to make `AsyncPredictor.serializer` and `AsyncPredictor.deserializer` into `@property`s that just read from and write to the inner `.predictor.(de)serializer`?
Contributor guide
Assessment
This issue has not been assessed yet.