Add deploy_targets field to GetLivestateRequest message #5618
Conversation
Signed-off-by: Shinnosuke Sawada-Dazai <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5618 +/- ##
=======================================
Coverage 26.57% 26.57%
=======================================
Files 475 475
Lines 50592 50592
=======================================
Hits 13444 13444
Misses 36085 36085
Partials 1063 1063 ☔ View full report in Codecov by Sentry. |
| @@ -30,6 +30,7 @@ service LivestateService { | |||
| message GetLivestateRequest { | |||
| string application_id = 1; | |||
| grpc.plugin.common.v1alpha1.DeploymentSource deploy_source = 2; | |||
| repeated string deploy_targets = 3; | |||
There was a problem hiding this comment.
[ask] Are these fields required or not?
There was a problem hiding this comment.
I think the application ID seems required, but the others are plugin-dependent.
There was a problem hiding this comment.
IMO, all should be required since this is livestate, so common plugins (like wait, wait approval) would not use this; only platform-based plugins use these fields, so should mark as required, wdyt?
There was a problem hiding this comment.
I agree with you about the plugins I can imagine now. But I think we should not restrict the possibility of third-party plugins that don't use these fields.
What this PR does:
as title
Why we need it:
We need the deploy target information to retrieve live resource states.
Which issue(s) this PR fixes:
Part of #5363
Does this PR introduce a user-facing change?: No.