-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat(outputs.mqtt): add support for MQTT 5 publish properties #12678
Conversation
solves: influxdata#12526 Add optional support for MQTT 5 publish properties in the config.
FYI @svagner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to put this together! I have a couple of changes and comments in-line.
Co-authored-by: Joshua Powers <powersj@fastmail.com>
Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank for the quick turn around!
@serroba you were the one who filed the feature request, could you give the artifacts in this PR a try and confirm they resolve your request?
I've verified this is working as expected by having With:
and the new config properties:
we get the expected values from the server:
@serroba and I work together :) |
Thanks for checking with me @powersj Changes LGTM. Thanks a lot to both! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks for the contribution @jlordiales!
resolves: #12526
Add optional support for MQTT 5 publish properties in the config