-
Notifications
You must be signed in to change notification settings - Fork 149
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 planner to use pipeline even for the first deployment when alwaysUsePipeline was configured #2472
Conversation
…aysUsePipeline was configured
/lgtm |
pkg/app/piped/planner/ecs/ecs.go
Outdated
@@ -81,6 +81,14 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu | |||
return | |||
} | |||
|
|||
// Force to use pipeline when the alwaysUsePipeline field was configured. |
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.
nits, how about moving the check to perform QuickSync in case of no pipeline configured at line L101 above this block and remove cfg.Pipeline != nil
condition check?
Of course, it's up to you.
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.
Sure. Nice catch! 👍
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.
@khanhtc1202 Applied your suggestion!
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 👍
@@ -81,6 +81,14 @@ func (p *Planner) Plan(ctx context.Context, in planner.Input) (out planner.Outpu | |||
return | |||
} | |||
|
|||
// Force to use pipeline when the alwaysUsePipeline field was configured. |
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 |
/changelog |
@nghialv: Changelog has been generated in response to this comment. DetailsChangelog since v0.16.0Notable ChangesNo notable changes for this release Internal Changes
|
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: