-
Notifications
You must be signed in to change notification settings - Fork 85
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(adr): addition of north-south messaging ADR #656
feat(adr): addition of north-south messaging ADR #656
Conversation
fix: edgexfoundry#640 Signed-off-by: jpwhitemn <[email protected]>
Signed-off-by: jpwhitemn <[email protected]>
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.
The proposal makes sense to me.
I do wonder about the mapping that the command service has to do, to transform the topics on request and on response; I think an example of the topic path on the northside used on request and response would be helpful, eg for the sensor01 example.
Is it the case that the request topic must include the device name and resource name, so the command service just has to map to the correct device service?
Would it make sense to echo the command name into the response, as a reality check?
Is it acceptable for more than one response to be published by the device service on the same correlation ID? Eg, send back "Acknowledged", then "Scheduled", then "Starting", then "Done" statuses? |
Should probably state if the burden is on the northside to manage timeouts or resending if no response - ie, the command service is "best effort" and will not do these things. |
Signed-off-by: jpwhitemn <[email protected]>
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.
Please see comments inline.
One global question, will it be possible to send/receive binary data (e.g. CBOR) using this approach?
Signed-off-by: jpwhitemn <[email protected]>
addressed the internal/external message bus issue cleaned up message envelope vs payload issue Signed-off-by: jpwhitemn <[email protected]>
Signed-off-by: jpwhitemn <[email protected]>
…ge comms section Signed-off-by: jpwhitemn <[email protected]>
Signed-off-by: jpwhitemn <[email protected]>
…er core working group meeting of 4/7/22 Signed-off-by: jpwhitemn <[email protected]>
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 believe due diligence has been done on this one. Let's send this up for a vote.
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 much better, but I still have a few comments inline...
Signed-off-by: jpwhitemn <[email protected]>
19b473f
…ions is still an open issue left for implementation per TSC meeting of 4/27/22 Signed-off-by: jpwhitemn <[email protected]>
…pproved ADRs Signed-off-by: jpwhitemn <[email protected]>
a1f9f9a
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. As approved on Core WG calls
…ge-bus Signed-off-by: edgex-jenkins <[email protected]>
fix: #640
Signed-off-by: jpwhitemn [email protected]
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-docs/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements: