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(controller): Fixes resource version misuse. Fixes #4714 #4741
Conversation
Signed-off-by: Alex Collins <alex_collins@intuit.com>
@@ -74,7 +74,8 @@ func waitOnOne(serviceClient workflowpkg.WorkflowServiceClient, ctx context.Cont | |||
req := &workflowpkg.WatchWorkflowsRequest{ | |||
Namespace: namespace, | |||
ListOptions: &metav1.ListOptions{ | |||
FieldSelector: util.GenerateFieldSelectorFromWorkflowName(wfName), | |||
FieldSelector: util.GenerateFieldSelectorFromWorkflowName(wfName), | |||
ResourceVersion: "0", |
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.
"0" means, start from the latest version.
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.
Nifty trick
@@ -210,13 +211,7 @@ func (s *workflowServer) WatchWorkflows(req *workflowpkg.WatchWorkflowsRequest, | |||
return nil | |||
case event, open := <-watch.ResultChan(): | |||
if !open { | |||
log.Debug("Re-establishing workflow watch") |
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.
as the client must have re-connection logic, we should not try and mask/mitigate the problem here
Signed-off-by: Alex Collins <alex_collins@intuit.com>
@@ -43,7 +43,22 @@ export default { | |||
return Observable.create((observer: Observer<any>) => { | |||
const eventSource = new EventSource(url); | |||
eventSource.onmessage = x => observer.next(x.data); | |||
eventSource.onerror = x => observer.error(x); | |||
eventSource.onerror = x => { | |||
switch (eventSource.readyState) { |
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.
some users are creating "unknown error" issues - lets provide clearer error message
Signed-off-by: Alex Collins alex_collins@intuit.com
Checklist: