yep, that's another area where there is still room for improvement. El mar, 24 oct 2023 a las 15:54, Alex Porcelli (<[email protected]>) escribió:
> Walter, > > I'm ok with starting with unit tests, but I don't think this solves > the problem. And with which a complex combination of factors (format, > different models, etc) it's easy to get this happening again. > > Would be great if we could come up with a better testing solution to > prevent this from happening again. > > On Tue, Oct 24, 2023 at 9:48 AM Walter Medvedeo <[email protected]> > wrote: > > > > Hi Alex, that's a good question. > > > > For this particular case, I think that a good start point is by adding > unit > > tests that ensure both binary and structured mode works, if we do that, > we > > don't need to set any complex infrastructure configuration, or avoid as > > much as we can. > > > > Let me show you an example: > > > > > https://github.com/apache/incubator-kie-kogito-apps/blob/7f8d673be331e0d2c3e3e5336219929dccee5f8c/jobs-service/jobs-service-messaging-http/src/test/java/org/kie/kogito/jobs/service/messaging/http/CloudEventConverterTest.java > > > > In that case, in a similar converter, we are checking that both modes are > > working. No need for a more complex infrastructure setup, because > > we emulate the information produced in both formats and ensure that the > > converter works properly. And yes, not perfect, but keeps the processing > > more covered. > > > > > > Cheers, > > Walter. > > > > Here's another example, a different part, where we emulate binary and > > structured mode as part of the unit tests. > > > > > https://github.com/apache/incubator-kie-kogito-apps/blob/7f8d673be331e0d2c3e3e5336219929dccee5f8c/jobs-service/jobs-service-common/src/test/java/org/kie/kogito/jobs/service/resource/v2/ExternalResourcesMock.java > > > > > > > > > > El mar, 24 oct 2023 a las 13:40, Alex Porcelli (<[email protected]>) > > escribió: > > > > > Thank you Walter to bring this to our attention! > > > > > > Now, my question is: how can we prevent such problem in the future? > > > > > > Clearly the existing testing infrastructure is not capturing well > enough to > > > prevent us to misstep again in future. > > > > > > On Tue, Oct 24, 2023 at 7:38 AM Enrique Gonzalez Martinez < > > > [email protected]> wrote: > > > > > > > I think is fair point. From my perspective there is no problem. In > the > > > end > > > > we just need to duplicate fields in what u call binary format and > unwrap > > > > those changes in the data index to make it work. > > > > > > > > Unfortunately the events requires need the context of the event as > it is > > > > big event was replace by raw diff events. > > > > > > > > My guess is that knative unwrap the cloud event and send that wrapped > > > data > > > > (otherwise) there is no eventpublisher which sends no cloud event > afaik.. > > > > is that correct ? > > > > > > > > El mar, 24 oct 2023, 13:25, Walter Medvedeo <[email protected]> > > > > escribió: > > > > > > > > > Hello Guys, > > > > > > > > > > I just wanted to raise up a tricky part of the cloud events that I > > > think > > > > > it's important to take into account when we introduce > modifications on > > > > the > > > > > events produced / received by our infrastructure in general. > > > > > > > > > > When we send cloud events over the wire, there are basically two > modes > > > > that > > > > > can be used to send them, "structured" and "binary", all good. In > > > > general, > > > > > every time our code send those events, it does by using the > > > "structured" > > > > > mode. And, in general, our services like the data-index, and the > > > > > jobs-service will receive them in "structured" mode too. > > > > > > > > > > For instance, if we use kafka, and the process produces an event, > that > > > > > event will be delivered in the "structured" mode to the data-index. > > > > > > > > > > BUT, there are some scenarios, where even when we produce these > events > > > > in " > > > > > structured" mode, we don't have 100% control, on which mode they > will > > > be > > > > > delivered. That is the case of the knative eventing infrastructure. > > > When > > > > we > > > > > have a installation that relies on knative eventing, very > simplified, > > > we > > > > > have something like this: > > > > > > > > > > Process --> (produce event in "structured" mode) --> knative broker > > > > > (receives the event) --> knative trigger (delivers the event in > > > "binary" > > > > > mode) --> Data Index (receives the event in binary mode) > > > > > > > > > > To deliver the originally event created in structured mode, in > binary > > > > mode > > > > > instead, is something internal to the knative eventing system, and > they > > > > > have good reasons for that. On reason, is for example, that by > managing > > > > the > > > > > events in the binary mode internally, it's easier to do > > > filtering/routing > > > > > operations, based on the ce-xxx headers that are used by the binary > > > mode. > > > > > > > > > > Why I am commenting this? > > > > > > > > > > As a side effect of the "binary" mode delivery, we must warrantee > that > > > > our > > > > > services, like Data Index, and Jobs Service a prepared to > understand > > > > those > > > > > events in both formats. > > > > > > > > > > Why? well, because if our architecture is deployed by using the > knative > > > > > eventing infrastructure (instead of kafka), those events will > arrive in > > > > > binary mode. > > > > > > > > > > Not sure if I could explain the "problem" clearly, but I'm happy > to do > > > a > > > > > demo, or whatever of a full setup in the knative system, and show > the > > > > > involved components to ensure the events delivery, etc, np. > > > > > > > > > > Now, going the concrete situation, I think this last change in the > > > events > > > > > reception at the data-index: > > > > > https://github.com/apache/incubator-kie-kogito-apps/pull/1881, is > > > > > introducing an issue when we are in knative eventing system. (and > we > > > > > receive the events in the binary format) > > > > > > > > > > Before, far from perfect there was kind of "tricky" processing that > > > tried > > > > > to couple with both formats, and more or less worked. Now after the > > > > change, > > > > > I can see some errors when we are in knative. > > > > > > > > > > And in parallel, we had an open PR > > > > > https://github.com/apache/incubator-kie-kogito-apps/pull/1889, > with > > > more > > > > > improvements in particular on the treatment on this events to > improve > > > > this > > > > > "tricky" code I was mentioning. > > > > > > > > > > So we had a kind of too many changes at the same piece of the pie, > > > > > basically. I think > > > > > https://github.com/apache/incubator-kie-kogito-apps/pull/1889 > should > > > > > probably have been merged first since it provided an improvement > on the > > > > > binary/structured, and thus, cloud be used for the processing of > the > > > new > > > > > events defined in the other PR. > > > > > > > > > > To be clear, it's no a complain, but I wanted to introduce the > topic > > > > > because I think it's important to keep in mind that both "binary" > and > > > > > "structured" mode for future changes. > > > > > > > > > > I'll now try to resolve all the merge conflicts on the PR and see > how > > > to > > > > be > > > > > sure both modes continue working. > > > > > > > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
