feat(transforms): Add Topic Formatting Capability to MQTTSecretSender #872
Conversation
AlexCuse
force-pushed
the
mqttsender-topic-formatting
branch
from
52a3866 to
317530a
Compare
AlexCuse
changed the title
Allow user to initialize MQTTSecretSender with a formatting function
that builds a final publish topic from the configured topic and the
input parameters to the MQTTSend function. Also change default behavior
to allow for placeholders for context values to be used in configured
topic, eg:
"topic/{context-key}" will publish to
"topic/current-value-at-context-key-for-this-pipeline-execution" on each call
Signed-off-by: Alex Ullrich <[email protected]>
AlexCuse
force-pushed
the
mqttsender-topic-formatting
branch
from
317530a to
bb3ec87
Compare
Codecov Report
@@ Coverage Diff @@
## master #872 +/- ##
==========================================
+ Coverage 62.33% 62.35% +0.01%
==========================================
Files 31 31
Lines 2031 2048 +17
==========================================
+ Hits 1266 1277 +11
- Misses 675 679 +4
- Partials 90 92 +2
Continue to review full report at Codecov.
|
| publishTopic, err = sender.topicFormatter(publishTopic, ctx, data) | ||
| } else { | ||
| for k, v := range ctx.GetAllValues() { | ||
| publishTopic = strings.Replace(publishTopic, fmt.Sprintf("{%s}", k), v, -1) |
There was a problem hiding this comment.
Would using text/template make sense here instead of the replace?
There was a problem hiding this comment.
But this approach does have an advantage, since text/template wasn't working well with special key names such as those containing a "$" character.
There was a problem hiding this comment.
I thought it would be best to keep the complexities of text/template (like you mention) out of the default behavior. It would require more complex expressions to render against the context map as well, and wouldn't provide a way for us to catch "replacement not found" errors that I can see. Have a look here:
https://play.golang.org/p/QwUSeJnLaTh
That said I did add the ability to specify your own topic format function as well, so if you wanted to do something with template that fits your use case it should be easy enough to plug in.
There was a problem hiding this comment.
It was a good idea to include the custom formatting function.
A simpler template format and finding replacement errors can be managed with text/template as here:
https://play.golang.org/p/QUJb5aA0IbF
btw, looking at your example, I realized that the index format which you used also does allow referencing names which special characters (e.g. {{ index . "$device" }}).
There was a problem hiding this comment.
From a end user perspective, I like the simplicity of the base-topic/{profilename}/device/{devicename}/state syntax.
There was a problem hiding this comment.
Okay, yes it is simpler from the user's perspective. I'll update my implementation in the MQTT trigger to use Alex's method.
Allow user to initialize MQTTSecretSender with a formatting function
that builds a final publish topic from the configured topic and the
input parameters to the MQTTSend function. Also change default behavior
to allow for placeholders for context values to be used in configured
topic, eg:
"topic/{context-key}" will publish to
"topic/current-value-at-context-key-for-this-pipeline-execution" on each call
Signed-off-by: Alex Ullrich [email protected]
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently MQTTSender publishes directly to the topic string configured.
Issue Number:
#871
What is the new behavior?
MQTTSender will replace {my-key} with the value found in the context at my-key by default. It will also be possible to configure a custom formatting function.
Does this PR introduce a breaking change?
Are there any new imports or modules? If so, what are they used for and why?
Are there any specific instructions or things that should be known prior to reviewing?
Other information