Handle port forwarding rules in sequence#4053
Merged
dgageot merged 1 commit intoGoogleContainerTools:masterfrom Apr 30, 2020
Merged
Handle port forwarding rules in sequence#4053dgageot merged 1 commit intoGoogleContainerTools:masterfrom
dgageot merged 1 commit intoGoogleContainerTools:masterfrom
Conversation
First, user defined rules. Then, automatic rules. Fixes GoogleContainerTools#3563 Signed-off-by: David Gageot <david@gageot.net>
dgageot
requested review from
balopat,
briandealwis,
nkubala and
tejal29
as code owners
Codecov Report
|
tejal29
reviewed
Apr 29, 2020
Contributor
tejal29
left a comment
tejal29
left a comment
There was a problem hiding this comment.
LGTM with a suggestion on UX
| for _, r := range resources { | ||
| r := r | ||
| go func() { | ||
| go func() { |
Contributor
There was a problem hiding this comment.
Not sure, if we are slowing down skaffold portforward phase due to sequentially forwarding resources.
Other alternative would be at L63, we first port forward uderDefinedResources in parallel. Wait for it to complete and then portforward serviceResources.
I don't mind this approach either "Correctness Vs Performance"
Contributor
Author
There was a problem hiding this comment.
Thanks for the idea @tejal29. You know what, let's go with correctness and if we have an issue, we'll improve the perf.
tejal29
approved these changes
Apr 29, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First, user defined rules.
Then, automatic rules.
Fixes #3563
Signed-off-by: David Gageot david@gageot.net