-
Notifications
You must be signed in to change notification settings - Fork 13
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 patch envelope status #32
Fix patch envelope status #32
Conversation
Since ZInfoPatchEnvelopeUsage itself is repeated repeating ZInfoPatchEnvelopeApp is redundant Signed-off-by: Pavel Abramov <uncle.decart@gmail.com>
Signed-off-by: Pavel Abramov <uncle.decart@gmail.com>
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.
Given that we do not yet have any user feedback on the PE usability at scale I'm far from certain that a single PE per app instance will be sufficient. Thus it sounds a bit counter-productive to change it from repeated to a single item. If it turns out that we only need one the API with repeated still works.
Should we chat about this with @gkodali-zededa ?
Sure @eriknordmark I think it makes sense |
@eriknordmark I think the question from Udit that lead to this PR is as follows |
Ah - in that case this change makes sense. |
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.
LGTM
Yetus correctly flags this as an incompatible change so we can only do this because we know that none of the controllers have yet implemented the patch envelop part of the API.
Since ZInfoPatchEnvelopeUsage itself is repeated repeating ZInfoPatchEnvelopeApp
is redundant
Status handling is not merged in EVE so it doesn't break anything in EVE code