airyhq / airy

💬 Open Source App Framework to build streaming apps with real-time data - 💎 Build real-time data pipelines and make real-time data universally accessible - 🤖 Join historical and real-time data in the stream to create smarter ML and AI applications. - ⚡ Standardize complex data ingestion and stream data to apps with pre-built connectors
https://airy.co/docs/core
Apache License 2.0
369 stars 44 forks source link

Add message accessor semantics #279

Closed chrismatix closed 3 years ago

chrismatix commented 3 years ago

Current state We have implicit semantics on the generated message avro class. E.g. updatedAt == null implies that this is a new message.

In order to allow for safe interactions with the message object across sources and core apps we need an accessor abstraction.

_Originally posted by @lucapette in https://github.com/airyhq/airy/pull/275#discussion_r521893265_

lucapette commented 3 years ago

The first approach that comes to mind is backend/lib/message that could expose something like MessageRepository.updateState(Message message,DeliveryState newState); (naming all up for discussion)

chrismatix commented 3 years ago

It's good for me, but it feels like it's far from complete, because remembering that updateState exists requires as much domain knowledge as knowing how to set these fields (for now).

So it's good, but long term I think we have to customize our avro generator pipeline 🙏

paulodiniz commented 3 years ago

I like the Repository approach, as long as we encapsulate all logic of that object (including building it)

lucapette commented 3 years ago

It's good for me, but it feels like it's far from complete, because remembering that updateState exists requires as much domain knowledge as knowing how to set these fields (for now).

So it's good, but long term I think we have to customize our avro generator pipeline 🙏

while it's true required knowledge is basically the same, this would still be a little less error-prone (and also introduce some sort of new pattern)

I like the Repository approach, as long as we encapsulate all logic of that object (including building it)

I'm not 100% about the building part as the Avro builder seems pretty good. I would argue we have a sketch idea of what we want to do so I will remove the needs discussion label and move it to do. We can sort out details in the pull request

chrismatix commented 3 years ago

I'm not 100% about the building part as the Avro builder seems pretty good. I would argue we have a sketch idea of what we want to do so I will remove the needs discussion label and move it to do. We can sort out details in the pull request

If at some point we want to do a full builder I would prefer that we extend the avro tools a bit. It's doable.

So for now I would do it the cheap way: backend/avro/communication/message hosts a java class MessageRespository that does what @lucapette described. We replace all direct state updates with usages of that class.

Is this sketch detailed enough?

lucapette commented 3 years ago

yes! that's why I had removed the needs discussion label. The details can be figured out in the PR itself