Halibut icon indicating copy to clipboard operation
Halibut copied to clipboard

add routing support

Open willemda opened this issue 5 years ago • 6 comments

Following https://github.com/OctopusDeploy/Halibut/issues/112, we've been working quite a bit on the routing feature on our side and would love to share our progress with you guys and discuss how that could be included in the main Halibut repository.

We changed approach and made use of the existing MessageEnvelope object, moving it up layers and adding routing information into it.

This approach doesn't require the "router" to deserialize the message into an object, avoiding necessity to update/maintain contracts on this part of the infrastructure. It also maintains backward compatibility with previous versions of Halibut as we've only added optional properties in the MessageEnvelope (backward compatibility is one way at the moment but could be made two ways I imagine).

One major downside is that we had to get rid of the DataStream support as this would have required the Router to deserialize the message in order to read DataStream information. We're thinking about re-introducing support for those, maybe by parsing a JObject instead of trying to deserializing the message. Any idea is most welcome here.

willemda avatar Apr 20 '20 14:04 willemda

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 20 '20 14:04 CLAassistant

@droyad and @uglybugger , apologies for the tags here but we're finalizing an approach for this internally including some routing security. Any input or direction? We'd love to contribute/change our approach to keep matching the official Halibut library.

janpieterz avatar Oct 06 '20 07:10 janpieterz

Hi @janpieterz, and thanks for the tag! I've only dealt peripherally with Halibut on the connection/retry policy side of things so I'll defer to @droyad on this one. I've also put the word out internally to see who else might have capacity to take a closer look.

uglybugger avatar Oct 11 '20 22:10 uglybugger

At first glance removing DataStream is a problem for us. We use it to transfer binary data (zips) in Octopus. I'll have to sit down with this to understand the implications.

droyad avatar Oct 12 '20 22:10 droyad

Correct, we didn't use it and it was a lot easier to cut support for that. @willemda I remember us chatting about this, was there a potentially easy way to do this? We ran into issues with deserializing the message on the routers for that right?

janpieterz avatar Oct 15 '20 06:10 janpieterz

With DataStreams the Stream information (id + length) is serialized in the message (which we can't deserialize on the router because the router doesn't have the reference to these objects).

We could query the a JObject directly on the router to get these DataStreams, read & write the stream to disk, and then retransmit these DataStreams back to their final destination (but this loses efficiency).

If we drop backward compatibility we could look at other ways of doing this by including the DataStream information in the Envelope (but I believe we'll still hit a loss in efficiency since we need to write the stream to disk on the router).

willemda avatar Oct 15 '20 15:10 willemda

Closing as we're having another approach that's more "sane" and doesn't require a fork

willemda avatar Nov 07 '23 19:11 willemda