-
Notifications
You must be signed in to change notification settings - Fork 618
Update request reply types #8698
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
Update request reply types #8698
Conversation
Signed-off-by: Calum Murray <cmurray@redhat.com>
Signed-off-by: Calum Murray <cmurray@redhat.com>
Signed-off-by: Calum Murray <cmurray@redhat.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8698 +/- ##
==========================================
- Coverage 51.58% 51.56% -0.02%
==========================================
Files 401 401
Lines 25431 25446 +15
==========================================
+ Hits 13119 13122 +3
- Misses 11509 11523 +14
+ Partials 803 801 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Calum Murray <cmurray@redhat.com>
| RequestReplyConditionTriggers apis.ConditionType = "TriggersReady" | ||
| RequestReplyConditionAddressable apis.ConditionType = "Addressable" | ||
| RequestReplyConditionEventPoliciesReady apis.ConditionType = "EventPoliciesReady" | ||
| RequestreplyConditionBrokerReady apis.ConditionType = "BrokerReady" |
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.
Lets make this consistent, and do CamelCase -> RequestReplyConditionBrokerReady
| Kind: "Broker", | ||
| Name: "broker", | ||
| }, | ||
| Timeout: ptr.To("PT30S"), |
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.
should we add test for invalid TO as well?
Signed-off-by: Calum Murray <cmurray@redhat.com>
creydr
left a comment
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
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Cali0707, creydr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
While implementing the request reply control plane, a few changes were needed in the request reply types for the feature to work correctly. This PR has those type changes.
Proposed Changes