search

lf-edge / eve-api

Repository for eve-api code
Apache License 2.0
0 stars 13 forks source link

Fix patch envelope status #32

Closed uncleDecart closed 1 year ago

uncleDecart commented 1 year ago

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

uncleDecart commented 1 year ago

Sure @eriknordmark I think it makes sense

gkodali-zededa commented 1 year ago

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 ?

@eriknordmark I think the question from Udit that lead to this PR is as follows We already have a repeated ZInfoPatchEnvelopeUsage inside ZInfoApp. Why do we need another repeated ZInfoPatchEnvelopeApp inside ZInfoPatchEnvelopeUsage. We can leave the repeated ZInfoPatchEnvelopeUsage inside ZInfoApp for the case when we have multiple PEs attached. Will we also need the repeated ZInfoPatchEnvelopeApp?

eriknordmark commented 1 year ago

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 ?

@eriknordmark I think the question from Udit that lead to this PR is as follows We already have a repeated ZInfoPatchEnvelopeUsage inside ZInfoApp. Why do we need another repeated ZInfoPatchEnvelopeApp inside ZInfoPatchEnvelopeUsage. We can leave the repeated ZInfoPatchEnvelopeUsage inside ZInfoApp for the case when we have multiple PEs attached. Will we also need the repeated ZInfoPatchEnvelopeApp?

Ah - in that case this change makes sense.