Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure extra events are serializable before uploading #20

Merged
merged 7 commits into from
Dec 13, 2023
Merged

ensure extra events are serializable before uploading #20

merged 7 commits into from
Dec 13, 2023

Conversation

gorkem2020
Copy link
Contributor

extra events may include objects that are not serializable by msgpack and results in failure when trying to upload. Adding json.loads/dumps ensures that each extra event is serializable before uploading, so that msgpack can process

extra events may contain raw objects some of which are not serializable by msgpack. Adding json.load ensures that the frame can be sent without fail.
@curusarn
Copy link
Contributor

curusarn commented Dec 6, 2023

Hi @gorkem2020,

Thank you for submitting the PR.
We'll merge this and release a new version soon!

@PetrHeinz
Copy link
Member

Hello @gorkem2020 and thanks again for the contribution! 🙌 I've noticed that your change doesn't really work for values passed in the context (only for extra). I've patched it up and updated tests.

Too late I realized that you had opened a PR from your master brach - I've pushed directly into it.
I hope that's alright and you're using the fork just to create this PR.

- formatter can still work its encoding changes
- no test case update needed, only added test cases
- end user functionality still remains the same (objects are serialized)
@PetrHeinz PetrHeinz merged commit 32f8507 into logtail:master Dec 13, 2023
5 checks passed
@PetrHeinz
Copy link
Member

@gorkem2020 Thank you again for the contribution, I just released it as v0.2.9 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants