feat: Add functionality to pass custom serializer #1116
+73
−38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…se class This refactoring introduces a new BaseEventSerializer abstract base class that provides a common interface and shared functionality for JSON serialization. Key changes include: - Creating an abstract base class with common serialization methods - Moving shared serialization logic to the base class - Adding an abstract default method for object serialization - Implementing safe integer range checking - Updating references in other files to use the new base class
|
|
davidberenstein1957
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
5 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| @@ -60,7 +61,7 @@ def post(self, **kwargs) -> httpx.Response: | |||
| """Post the `kwargs` to the API""" | |||
| log = logging.getLogger("langfuse") | |||
| url = self._remove_trailing_slash(self._base_url) + "/api/public/ingestion" | |||
| data = json.dumps(kwargs, cls=EventSerializer) | |||
| data = json.dumps(kwargs, cls=self._serializer) | |||
There was a problem hiding this comment.
Ensure _serializer is initialized (e.g. self._serializer = EventSerializer) in init as it's used in json.dumps but not set.
There was a problem hiding this comment.
LGTM
5 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce BaseEventSerializer as an abstract base class
This refactoring introduces a new BaseEventSerializer abstract base class that provides a common interface and shared functionality for JSON serialization. Key changes include:
Creating an abstract base class with common serialization methods
Moving shared serialization logic to the base class
Adding an abstract default method for object serialization
Implementing safe integer range checking
Updating references in other files to use the new base class
documentation with explanation
JS SDK
Greptile Summary
Disclaimer: Experimental PR review
Introduces a new BaseEventSerializer abstract base class to standardize JSON serialization across the codebase, enabling custom serializer implementations while maintaining existing functionality.
_serializerfield inLangfuseClient.__init__inlangfuse/request.pyneeds to be added to prevent runtime errorsIngestionConsumer._serializerdefined as class variable could cause shared state issues with multiple instancesIngestionConsumerto configure custom serializers without subclassingBaseEventSerializerwith requireddefault()method inlangfuse/serializer.py💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!