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

Expose a way to serialize a KV with any Serde Serializer #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nightkr
Copy link

@nightkr nightkr commented Mar 12, 2020

This is useful if you need to serialize to a more structured form, such as sending full JSON blobs to a HTTP endpoint.

This is useful if you need to serialize to a more structured form, such as
sending full JSON blobs to a HTTP endpoint.
@ncalexan
Copy link

Hey! I came here to file a ticket saying "make it easier to log inline JSON, i.e., serde structs as log values". Since there seems to be lots of conflicting information and questions about how to do this on the web (several threads in gitter, for example) I made https://github.com/ncalexan/slog-nested-values-example to work out what I needed to do. But I think your PR would do this and a little more. Is that correct? Can we get some discussion of this?

I would definitely like to see an example and/or tests of this functionality, 'cuz it's not obvious that it exists without examples :)

@nightkr
Copy link
Author

nightkr commented Mar 30, 2020

@ncalexan This PR is for dumping log messages into arbitrary Serde Serializers (that is, output formats). If I understand you correctly, what you want is to be able to use Serde Serializes (serializable objects) as slog Values, which is more or less going the opposite way, and already works (as your example demonstrates).

@ncalexan
Copy link

@ncalexan This PR is for dumping log messages into arbitrary Serde Serializers (that is, output formats). If I understand you correctly, what you want is to be able to use Serde Serializes (serializable objects) as slog Values, which is more or less going the opposite way, and already works (as your example demonstrates).

Ah, I see -- I misunderstood the role of SerdeSerializer. I will open a new ticket. Thanks, @teozkr!

@dpc
Copy link
Contributor

dpc commented Mar 31, 2020

So, do I close this one?

@nightkr
Copy link
Author

nightkr commented Mar 31, 2020

@dpc Well, the PR is still relevant and exposes functionality that slog-json doesn't have otherwise. I should just have explained it better in the original body.

@@ -9,6 +9,7 @@ documentation = "https://docs.rs/slog-json"
homepage = "https://github.com/slog-rs/slog"
repository = "https://github.com/slog-rs/json"
readme = "README.md"
edition = "2018"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we can get away without it? Unless there's a really good reason, would rather not have these couple of folks stuck with older compiler or something, have to upgrade.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely. That's why I split the PR into cf7d07a (which adds the new serialize function) and e0a0f69 (which upgrades to 2018 without changing any functionality).

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