feat: builtin group measurements flow transformer#4201
Conversation
didier-wenzek
requested review from
a team,
albinsuresh,
jarhodes314,
reubenmiller and
rina23q
as code owners
didier-wenzek
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Robot Results
|
| Execute Command tedge mqtt pub collectd/localhost/temperature/temp1 "`date +%s.%N`:50" | ||
| Execute Command tedge mqtt pub collectd/localhost/temperature/temp2 "`date +%s.%N`:40" | ||
| Execute Command tedge mqtt pub collectd/localhost/pressure/pres1 "`date +%s.%N`:10" | ||
| Execute Command tedge mqtt pub collectd/localhost/pressure/pres2 "`date +%s.%N`:20" |
There was a problem hiding this comment.
Whilst having individual commands is much easier to read, it might result in some timing issues as each Execute Command has to do an underlying docker api call, and wait for the response, so I wouldn't be surprised if this could take longer than 1 second when the CI runner is under load.
There was a problem hiding this comment.
I'd suggest that we just update the flow interval and config.time_window to higher values so that we can write manual tests like this better.
There was a problem hiding this comment.
I decided to simply revert my change - i.e. to send all the collectd messages in a single Execute Command
| Execute Command tedge mqtt pub collectd/localhost/temperature/temp1 "`date +%s.%N`:50" | ||
| Execute Command tedge mqtt pub collectd/localhost/temperature/temp2 "`date +%s.%N`:40" | ||
| Execute Command tedge mqtt pub collectd/localhost/pressure/pres1 "`date +%s.%N`:10" | ||
| Execute Command tedge mqtt pub collectd/localhost/pressure/pres2 "`date +%s.%N`:20" |
There was a problem hiding this comment.
I'd suggest that we just update the flow interval and config.time_window to higher values so that we can write manual tests like this better.
| ```toml | ||
| [[steps]] | ||
| builtin = "group-measurements | ||
| interval = "1s" | ||
| config.time_window = "500ms" | ||
| ``` |
There was a problem hiding this comment.
Any specific reason to have both the in-line version as well as the standard array representations for just this keyword? The doc of other transformers only had the inline variant, hence asking.
There was a problem hiding this comment.
No specific reason.
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
A value created in JS using `new Date()` cannot be directly returned to the flow engine. A script has then to invoke `valueOf()` method to assign a date to a message timestamp. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
didier-wenzek
force-pushed
the
feat/builtin-group-measurements-flow-transformer
branch
from
c1dbed5 to
03aa29d
Compare
didier-wenzek
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
didier-wenzek
deleted the
feat/builtin-group-measurements-flow-transformer
branch
3 participants
Proposed changes
group-measurementsbuiltin flow transformerTypes of changes
Paste Link to the issue
This PR has been extracted from #4136, the point being to ease the release of a new version of tedge deprecating the collectd mapper
group-measurementsbuiltinChecklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments