-
Notifications
You must be signed in to change notification settings - Fork 12
Add initial version of TypeDB extension #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
76472d2 to
2581373
Compare
383143a to
b8cc216
Compare
b8cc216 to
582f992
Compare
239a962 to
203ff68
Compare
purcell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a number of changes here, rather than just leave a pile of comments. I like the ProxiedDockerContainerExtension and agree it could become part of core.
I took the liberty of rewriting the HTTP2->GRPC forwarding, to try and make the logic clearer. There's a commit there which documents the danger of calling that patch function twice, by making it safe to do so, but that's only relevant if the code is ever extracted and used elsewhere.
The extent of my testing here was running the existing tests.
|
In case it helps, here's the diff for the changes I added on top of yours: 1cc63b7...typedb-extension |
203ff68 to
e3c56b6
Compare
|
The main thing I thought of that may be missing for avid TypeDB users was the ability to mount a volume into the docker container that is started, so that that data could persist between runs: the command flags alone don't really allow for that. If that makes sense (I wasn't sure if it would be idiomatic for LS and its extensions), I have a commit waiting that adds this feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks @purcell for proactively pushing the additional commits. 🚀 I've looked at the delta, and your changes look good to me 👍 (cannot approve my own PR, but ✅ )
The main thing I thought of that may be missing for avid TypeDB users was the ability to mount a volume into the docker container that is started, so that that data could persist between runs: the command flags alone don't really allow for that. If that makes sense (I wasn't sure if it would be idiomatic for LS and its extensions), I have a commit waiting that adds this feature.
That's a great point! I guess we could either (1) add a config option (via an environment variable passed to LocalStack) to mount a local directory into the TypeDB container at /var/lib/typedb/data, or (2) add a config flag to enable persistence, and then use the LocalStack Volume as the persistence volume mounted into the TypeDB container. (haven't tested this locally, but should technically be possible to mount a single host folder into multiple containers). Given that there are different options to evaluate, should we perhaps tackle this as a follow-up PR?
There's a commit there which documents the danger of calling that patch function twice, by making it safe to do so, but that's only relevant if the code is ever extracted and used elsewhere.
Regarding the double-patching - great catch, thanks for adding the safeguard here. In my view, we could also remove the patch_counter, and simply raise an exception if an attempt is made to apply the patches multiple times (which, as you mentioned, is inadvisable or could point at some issue in the code logic anyway). That would make things a bit easier and more explicit - wdyt?
Motivation
Add initial version of TypeDB extension. Migrating from the temporary repo used to build out the MVP, in collaboration with the TypeDB team: https://github.com/whummer/localstack-utils/tree/master/localstack-typedb
After installing the extension, a TypeDB server instance will become available under
typedb.localhost.localstack.cloud:4566, allowing you to create and manage TypeDB databases directly from your AWS applications running in LocalStack.For example, you could create a microservice backed by a Lambda function that connects to a TypeDB database upon invocation. See here for a simple example application that makes use of this extension.
This is part of a collaboration / co-branding with TypeDB, a blog post should be following soon.. (/cc @flyingsilverfin )
Changes
ProxiedDockerContainerExtensionwhich can be used to define an Extension backed by a Docker container. We may be able to pull out this class into common utils over time.ProxyResourceclass which is used as a resource that hooks into the LocalStack (rolo) handler chain and forwards matching requests to the extension containerProxyRequestMatcherclass which allows the extension to define which incoming requests are in scope and should be forwarded to the TypeDB containerTcpForwarderclass to enable a bidirectional TCP proxy tunnel that forwards traffic to and from the external containerTypeDbExtensionclass which defines the main entrypoint for the extension