-
Notifications
You must be signed in to change notification settings - Fork 5.8k
fix: provider output handling and watch rebuild re-invocation #13732
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -125,10 +125,10 @@ func (s *composeService) executePlugin(cmd *exec.Cmd, command string, service ty | |||||||||||||
| } | ||||||||||||||
| switch msg.Type { | ||||||||||||||
| case ErrorType: | ||||||||||||||
| s.events.On(newEvent(service.Name, api.Error, msg.Message)) | ||||||||||||||
| s.events.On(newEvent(service.Name, api.Error, firstLine(msg.Message))) | ||||||||||||||
| return nil, errors.New(msg.Message) | ||||||||||||||
| case InfoType: | ||||||||||||||
| s.events.On(newEvent(service.Name, api.Working, msg.Message)) | ||||||||||||||
| s.events.On(newEvent(service.Name, api.Working, firstLine(msg.Message))) | ||||||||||||||
| case SetEnvType: | ||||||||||||||
| key, val, found := strings.Cut(msg.Message, "=") | ||||||||||||||
| if !found { | ||||||||||||||
|
|
@@ -281,3 +281,12 @@ func (c CommandMetadata) CheckRequiredParameters(provider types.ServiceProviderC | |||||||||||||
| } | ||||||||||||||
| return nil | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // firstLine returns the first line of s, stripping any trailing newlines. | ||||||||||||||
| func firstLine(s string) string { | ||||||||||||||
| s = strings.TrimRight(s, "\n") | ||||||||||||||
| if i := strings.IndexByte(s, '\n'); i >= 0 { | ||||||||||||||
| return s[:i] | ||||||||||||||
|
Comment on lines
+287
to
+289
|
||||||||||||||
| s = strings.TrimRight(s, "\n") | |
| if i := strings.IndexByte(s, '\n'); i >= 0 { | |
| return s[:i] | |
| s = strings.TrimRight(s, "\r\n") | |
| if i := strings.IndexByte(s, '\n'); i >= 0 { | |
| return strings.TrimRight(s[:i], "\r") |
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.
New
SkipProvidersbehavior is introduced here, but there doesn’t appear to be any test exercising convergence when provider services are present andSkipProvidersis true. Given how central convergence is, adding a unit test that asserts provider services are not invoked (and dependent services still converge) would help prevent regressions.