fluffysquirrels / mqtt-async-client-rs

An MQTT client written in Rust
MIT License
40 stars 16 forks source link

Serialize/Deserialize for ClientBuilder #20

Open DrSloth opened 3 years ago

DrSloth commented 3 years ago

Thank you for writing this crate it is very nice to use and perfectly fits my tokio driven IoT needs. It would be nice though if the ClientBuilder would implement serdes Serialize/Deserialize optimally behind a feature gate. In my use case this feature would help a lot as i could read configurations from config files.

fluffysquirrels commented 3 years ago

Thanks for your interest in the crate, I'm so glad it fits your needs.

I think this is a good idea. One concern I have is that the current ClientBuilder/Client deliberately don't expose the user's password (e.g. through Debug) to try and make them foolproof to use without accidentally logging passwords or similar. Implementing just Deserialize is no problem, but implementing Serialize would potentially expose passwords. I suppose I could just document this and rely on the user's judgement.

What are your thoughts?

DrSloth commented 3 years ago

I think you are correct. A foolproof client builder would be nice, but i think this is an abstraction the user of your crate would be able to handle themselves if it is documented correctly. If a user sees the password output to console in debug print and doesn't think "oh it is a bad idea to do this in production" it is sort of their own fault. Another option would be to just exclude it from the normal serialization process with the #[serde(skip)] field attribute and let the user decide how they serialize those.

fluffysquirrels commented 3 years ago

I decided to implement Deserialize for ClientBuilder only (for now) under the "serde" feature. Hope that helps your use case and happy to discuss Serialize some more if you have a concrete use case.

fluffysquirrels commented 3 years ago

Whoops, going to have to re-think this one. Several referenced structs / enums within ClientBuilder are not Deserialize, including a rustls config type.