-
Notifications
You must be signed in to change notification settings - Fork 4
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
Improve current state #231
Conversation
Move functions to make scope clearer and easier to find
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 good, thanks for cleaning up a lot of the stuff I've just left here 😅 🚀
@@ -27,21 +27,39 @@ import ( | |||
|
|||
func (c K8Client) ApplyNetworkPatch() error { |
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.
💭
Nice that you got this working for SaaS too. I think the only thing missing would be to pause reconciliation. Same for all other functionality that modifies any of the resources managed by the ZeebeCluster
CRD.
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.
Jep right now it is only this I think the other stuff is inside the container like disconnecting
"k8s.io/client-go/util/retry" | ||
) | ||
|
||
func (c K8Client) PauseReconciliation() error { |
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.
❓ Is there something we could link as documentation for how reconciliation works?
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.
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.
Maybe we could add that for future people who may not know where this is defined (in case it needs to change) 🙃
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.
Thanks I added it 👍
Thanks for the review 🙇 |
Can be seen as preparation for working on disconnect gateways.
Don't get scared by the number of lines. It is mostly moving code and adding new tests.