Add support for alternate backends, including a NATS adapter#79
Add support for alternate backends, including a NATS adapter#79jgaskins wants to merge 4 commits intocable-cr:masterfrom
Conversation
There was nothing there anyway
| @@ -0,0 +1,48 @@ | |||
| require "turbo/cable" | |||
There was a problem hiding this comment.
Since Cable adopted a bunch of the customizations I'd made to my fork, I didn't have to make a single change to my turbo/cable integration! 💯
| #{Turbo.javascript_tag} | ||
| #{Turbo.cable_tag} | ||
| #{Turbo::Frame.new(id: "time") { }} | ||
| #{Turbo.stream_from "time"} |
There was a problem hiding this comment.
It blew my mind that this was all we needed to render in order to demo this. 🤯
| def close_subscribe_connection | ||
| nats.close rescue nil | ||
| end | ||
|
|
||
| def close_publish_connection | ||
| nats.close rescue nil | ||
| end |
There was a problem hiding this comment.
NATS multiplexes everything over a single TCP connection (no connection pool needed), so we ignore any errors caused by closing the client twice.
This might be too naive since NATS::Client#close also drains subscriptions (processes any pending messages and synchronous requests), which can also raise errors, so we may want this to be a bit more precise on what exceptions it swallows silently.
| getter streams = Hash(String, Set(NATS::Subscription)).new { |streams, channel| | ||
| streams[channel] = Set(NATS::Subscription).new | ||
| } |
There was a problem hiding this comment.
Had to use {} instead of do/end here due to precedence.
The reason we had to include this is because you can't unsubscribe from a NATS subject directly. You have to unsubscribe from the subscription id since you can subscribe to the same subject multiple times.
| register "redis" | ||
| register "rediss" |
There was a problem hiding this comment.
Registering which URI schemes you support is as simple as this.
| setting redis_ping_interval : Time::Span do | ||
| backend_ping_interval | ||
| end |
There was a problem hiding this comment.
I can't tell if Habitat actually supports the block format like getter does, but it didn't raise an error.
There was a problem hiding this comment.
oh, that's interesting... At this level, setting is a macro that takes a TypeDeclaration. That basically does this..
def self.{{ type_dec.var }} : {{ type_dec.type }}
{{ type_dec.value }}
end
A little more complex, but that basic idea. So I guess the question is, what does Crystal consider the block here? My guess is the block is passed to setting which never calls yield and so it's ignored 🤔
There was a problem hiding this comment.
Yeah, there's an open issue on it. luckyframework/habitat#54
There was a problem hiding this comment.
Very likely. When I looked at the code for the setting macro it looked like there was some indirection that I didn't know how to follow.
What I was aiming for here was something like the block for getter and property to lazily initialize the setting, so if you were using the Redis-specific setting name it would still work, but now that I write that out I'm thinking actually maybe we could just do methods for that instead of setting multiple ivars?
There was a problem hiding this comment.
Yeah, methods may be fine here. I'm updating habitat issues to track some of these things too.
| Cable.configure do |settings| | ||
| settings.route = "/cable" # the URL your JS Client will connect | ||
| # settings.url = "redis:///" | ||
| # settings.url = ENV.fetch("NATS_URL", "nats:///") | ||
| settings.url = ENV.fetch("CABLE_BACKEND_URL", "redis:///") | ||
| end |
There was a problem hiding this comment.
Notice that in the settings for the demo we aren't actually setting backend_class anywhere. Which backend is used is based entirely on url.
| @backend : BackendCore | ||
|
|
||
| def initialize | ||
| @backend = REGISTERED_BACKENDS[URI.parse(::Cable.settings.url).scheme].new |
There was a problem hiding this comment.
This is the magic behind the automatic runtime backend selection.
TODO: I want to provide a better error than a plain-old KeyError. I'm thinking maybe something like this:
raise Cable::UnknownBackend.new("Cannot find a backend for URI scheme #{scheme.inspect}. Did you require the adapter for it?")
| setting redis_ping_interval : Time::Span = 15.seconds | ||
| setting backend_class : Cable::BackendCore.class = Cable::RegistryBackend, example: "Cable::RedisBackend" | ||
| setting backend_ping_interval : Time::Span = 15.seconds | ||
| @[Deprecated("Use backend_ping_interval")] |
There was a problem hiding this comment.
I'd be curious to see if this shows... Since the method doesn't take annotations in to consideration, this may not actually render. That would be a nice feature for Habitat though.
There was a problem hiding this comment.
I think you're right. It looks like it collects the settings into a data structure and then emits them later instead of emitting them in-place, so this annotation may actually be attached to something else, which would be confusing, or nothing at all. 🤔
|
I love this update. This should also make it easier to add #49 too which I've been wanting to do. Looks like there's still a few things left to clean up before we go merging stuff, but it's a great start! |
Definitely. When I first looked at this I was trying to figure out how we'd make the backends pluggable and reduce dependency on Redis specifically. As an experiment*, I'm not using Redis at all in one of my services so I wanted to see if we could get Cable working with NATS so I wouldn't need to add it. 😄 * Trying to see if I could do all the things I typically use Redis for (sessions, caching, background jobs, and pub/sub) in NATS since it supports key/value storage as well as pub/sub and streams. So far, it's working really well. |
|
@jgaskins I've created a new shard for this https://github.com/cable-cr/cable-nats I didn't port any of your code over, but just started the initial shard structure. I hoping this also makes things easier to test. I'm gonna leave this PR open for now just so it's easier to find 😆 but I'll close it out eventually since we won't merge this in to this shard directly. |
2 participants
I haven't added have any specs because this started as just a proof of concept, and I think I broke existing specs, but I did include an example app that lets you choose between backends by changing only the configuration.
The way it works is by changing the default backend to a
RegistryBackend, and the individual backend implementations "register" themselves with it. Then when a client connects, it chooses the backend to use based on the URI scheme inCable.settings.url. You can still setsettings.backend_classto the specific implementation so it should remain backwards-compatible, but we couldn't leave the default asRedisBackendsince, in the case where the application doesn't use Redis, it wouldn't exist as a class and the app wouldn't compile. Since theRegistryBackendreferences the other backends at connection time based on configuration, we don't rely on any specific implementation as the default.