Disallow unserializable #408
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
It appears this PR is a release PR (change its base from master if that is not the case).
Here's a release checklist:
- Update package version
- Update
poetry.lock - Change PR merge option
- Update template repo
- Search for objects to be deprecated
- Test parts not covered with pytest:
- web_api tutorials
- Test integrations with external services (telegram; stats)
There was a problem hiding this comment.
I had to use model_construct when working with models that had pickle serialization utils because model_validate would try to use pickle_validator and fail.
Now that it is no longer use, we can replace model_construct calls with __init__.
Also: need to update docs (context guide, context api ref, slots guide, slots api ref, e.t.c.) to mention that it is forbidden to store unserializable types.
chatsky/core/message.py
Outdated
| original_message: Optional[Any] = None | ||
| annotations: Optional[Dict[str, Union[BaseModel, JsonValue]]] = None | ||
| misc: Optional[Dict[str, Union[BaseModel, JsonValue]]] = None | ||
| original_message: Optional[Union[BaseModel, JsonValue]] = None |
chatsky/slots/slots.py
Outdated
| if value is not None: | ||
| return pickle_validator(value) | ||
| return value | ||
| extracted_value: Union[BaseModel, JsonValue] |
There was a problem hiding this comment.
Slots store exceptions on failure (which are not serializable).
I think we should store exception representation instead.
chatsky/core/message.py
Outdated
| misc: Optional[Dict[str, Any]] = None | ||
| original_message: Optional[Any] = None | ||
| annotations: Optional[Dict[str, Union[BaseModel, JsonValue]]] = None | ||
| misc: Optional[Dict[str, Union[BaseModel, JsonValue]]] = None |
There was a problem hiding this comment.
Change type annotation (for Union[BaseModel, JsonValue]) to allow deeper BaseModel usage (e.g. a dictionary or a list with BaseModel values).
There was a problem hiding this comment.
PydanticValue: TypeAlias = Union[
List["PydanticValue"],
Dict[str, "PydanticValue"],
BaseModel,
str,
bool,
int,
float,
None,
]
| @@ -627,7 +627,7 @@ async def _on_event(self, update: Update, _: Any, create_message: Callable[[Upda | |||
| data_available = update.message is not None or update.callback_query is not None | |||
| if update.effective_chat is not None and data_available: | |||
| message = create_message(update) | |||
| message.original_message = update | |||
| message.original_message = update.to_dict(recursive=True) | |||
There was a problem hiding this comment.
This also needs to apply to extra fields in Attachments.
Add a validator for Attachment and Message extras that modifies the extra field via to_dict if the field is of the TelegramObject value.
AFAIK if the extra field value is a dictionary from to_dict it should still work for the tg bot methods.
Description
I checked it and to the best of my knowledge it should work now.
Checklist
List here tasks to complete in order to mark this PR as ready for review.
To Consider
.ignorefiles, scripts (such aslint), distribution manifest (if files are added/deleted)