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]
