Some more context: after discussion with Ole: I think I might have overstepped the authority in terms of "-2" - it is component maintainers' decision if they want to put even completely breaking change into their component that sets the world on fire - if the CI lets that through - it is their decision. My apologies for this - I changed my vote to "-1".
As for some substance: Looks like there are two issues in one commit (always useful to split even if they are small :) 1) incorrect message IDs used in replies. This, we could argue, would have been hard (if possible at all) to make work - I will let the govpp maintainers to prove me wrong, but at least my experience with Rust version of APIs - those are not usable. With my release manager hat on - I could be convinced to have this cherry-picked even into the stable branches. 2) the endianness of the fields in the nat44_ed_vrf_tables_send_details: here, the message ID looks correct, so one can make a conservative conjecture that an API user might have used the API, but "fixed" it on the client side. So the "fix" on the VPP side would trigger unexpected changes required on their side. Given the message is not marked experimental, this is a silent violation of what we promise about API compatibility. With my release manager hat on - I definitely would not like to see it cherry-picked to stable branches, even if maintainers decide it's "good enough" to have it merged to master (which I am also not a fan of, hence my now -1 :) The reason I am so "pig-headed" about the API changes is - every alternative-free modification creates "urgent" work downstream. I am sure the consumers have worked out the processes that it does not turn into "drop your friday night party with friends and fix it"-urgent, but it is still unplanned work. And we promise with "production" APIs that there will be no unplanned work in their maintenance. Hopefully it gives a bit more context and the background of my reasoning :-) --a On Wed, Mar 22, 2023 at 2:26 PM filvarga <filipvarg...@gmail.com> wrote: > Hi, > > Yes I fully agree with Andrew on changing APIs. Though the api was broken > in the first place. If used incorrect message is returned. Because of that > I am not even sure if it is functional at all. It is more likely that > nobody before addressed this issue because this feature is not used. > > So I would suggest either to set it as deprecated and add v2 which will be > flagged as experimental or change it on master because It might not be even > functional. > > Best regards, > Filip Varga > > > st 22. 3. 2023 o 14:11 Andrew Yourtchenko <ayour...@gmail.com> napísal(a): > >> Hey Daniel, >> >> I had a quick look and it looks like the APIs in question are also not >> marked as experimental ? >> >> If I am right, then to me seems like a silent behavior change to a >> production API - something we have heard repeated complaints from the >> downstream consumers for…. >> >> I had -2’d it for now - we should definitely discuss it here first; as is >> i would consider it a no-go even for a master, but I am happy to hear >> others opinions and be convinced otherwise :-) >> >> --a >> >> On 22 Mar 2023, at 10:36, Daniel Béreš <daniel.be...@pantheon.tech> >> wrote: >> >> >> >> Hello Guys, >> >> I wrote a short patch to fix a couple api handlers for nat44ed [1] . >> <https://gerrit.fd.io/r/c/vpp/+/38459>It was probably forgotten, so I'm >> reminding myself. >> >> I still have a question about whether it is possible to cherry-pick the >> patch to vpp v23.02 and 22.10. >> >> [1] https://gerrit.fd.io/r/c/vpp/+/38459 >> >> >> Thanks, >> Daniel >> >> >> >> >> >> >> > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#22749): https://lists.fd.io/g/vpp-dev/message/22749 Mute This Topic: https://lists.fd.io/mt/97774402/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/leave/1480452/21656/631435203/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
