-
Notifications
You must be signed in to change notification settings - Fork 485
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
fix: Add backwards compatability for Device System Events #4121
fix: Add backwards compatability for Device System Events #4121
Conversation
fixes edgexfoundry#4117 Signed-off-by: Leonard Goodell <leonard.goodell@intel.com>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Codecov Report
@@ Coverage Diff @@
## main #4121 +/- ##
=======================================
Coverage 43.66% 43.66%
=======================================
Files 120 120
Lines 10617 10617
=======================================
Hits 4636 4636
Misses 5594 5594
Partials 387 387
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
LGTM
# This will default to false if not provided in old config. Messagebus is now needed by Device System Events and Service Metrics | ||
# TODO: Remove this setting EdgeX 3.0 | ||
RequireMessageBus = true | ||
|
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.
Didn't get a chance to review this.
Couldn't we just check if MessageQueue
or MessageQueue.protocol
are set, then assume message bus is required?
edgex-go/cmd/core-metadata/res/configuration.toml
Lines 64 to 65 in e3ca38a
[MessageQueue] | |
Protocol = "redis" |
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.
I considered that, but that is ambiguous as to if it is missing because of old configuration or just bad configuration. I prefeed it to be explicate.
fixes #4117
Signed-off-by: Leonard Goodell leonard.goodell@intel.com
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:
describing the break)feat: Document Core Metadata Device System Events edgex-docs#830
Testing Instructions
Run Non-secure EdgeX Stack from compose builder
Stop Core Metadata
In Consul, remove
MessageQueue
configuration for Core MetadataUsing the branch for this PR
Remove
MessageQueue
andRequireMessageBus
configuration from TOML fileBuild Core Metadata docker image locally
Modify generated compose file to switch Core Metadata image to be local image
Restart local Core Metadata image using modified compose file
Verify Core Metadata logs don't show anything about MessageBus and started without error
Using Edgex UI delete a device
Verify that Core Metadata logs show warning that the Device System Event couldn't be published.
Stop Core Metadata image
Remove Core Metadata config from Consul
Restore local TOML file
Rebuild local Core Metadata image
Restart local Core Metadata image using modified compose file
Verify Core Metadata logs do show MessageBus connection info and started without error
Using Consul set Core Metadata log level to Debug
Using Edgex UI delete a device
Verify that Core Metadata logs show debug message that the Device System Event was published.
New Dependency Instructions (If applicable)