Skip to content

more flexible body handling #17

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

msutkowski
Copy link
Contributor

@msutkowski msutkowski commented Jan 21, 2024

@v0idpwn Just wanted to get this up for a discussion. We have changes very similar to this to support our desired usage on a fork, but I wanted to see how you would recommend we could support more serializer types before I go in a specific direction. As an example, we should able to handle json, avro, protobuf, etc, and we can't really do that while casting body as a string. And ideally we'd update docs to show "if you want human readable entries in the database, do X, if not, do nothing"

Any guidance on the preferred implementation would be 💯, thanks!

@v0idpwn
Copy link
Owner

v0idpwn commented Jan 22, 2024

Hi, thanks for the PR!

Are all of those those :brod.value()? If not, then I assume you're using a different adapter? Could you share a bit more about how these values look like?

@msutkowski
Copy link
Contributor Author

Hi, thanks for the PR!

Are all of those those :brod.value()? If not, then I assume you're using a different adapter? Could you share a bit more about how these values look like?

Yea, they'd just be passed straight through as iodata()

@v0idpwn
Copy link
Owner

v0idpwn commented Jan 24, 2024

Perhaps we can then be a bit more strict? I'm thinking of saving it in the database as binary after IO.iodata_to_binary/1. Should be cheaper than doing binary_to_term and back, and feels a bit better. Still, if you think we should support more data-types, I'd be open :)

@v0idpwn
Copy link
Owner

v0idpwn commented Apr 17, 2024

Ping @msutkowski :)

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.

2 participants