AWS Fargate gRPC cluster example #5
Conversation
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Thanks for your contribution @igorshmukler! I left some questions 🙇🏻
| ARG SPICEDB_GRPC_PRESHARED_KEY | ||
| ENV SPICEDB_GRPC_PRESHARED_KEY=${SPICEDB_GRPC_PRESHARED_KEY} | ||
| ARG SPICEDB_DATASTORE_ENGINE | ||
| ENV SPICEDB_DATASTORE_ENGINE=${SPICEDB_DATASTORE_ENGINE} | ||
| ARG SPICEDB_DATASTORE_CONN_URI | ||
| ENV SPICEDB_DATASTORE_CONN_URI=${SPICEDB_DATASTORE_CONN_URI} |
There was a problem hiding this comment.
🤔 it's not clear to me why this has to be added. Why can't those be specified as environment variables that get injected in the container via the cloudformation template? This container definition does not seem to provide anything the upstream couldn't provide
There was a problem hiding this comment.
sorry, i don't understand the comment. don't know the project as well as you probably do. is the goal of your question to understand whether my approach is the only approach possible? please clarify.
There was a problem hiding this comment.
Let me try again 😄 What I'm trying to say is that theoretically you wouldn't need to define your own Dockerfile and use the images we the Authzed team push to public registries like Dockerhub, Quay or GitHub Container Registry. My goal is to see if we can remove this Dockerfile and instead adjust the cloudformation accordingly to inject the corresponding environment variables. Does that make sense?
There was a problem hiding this comment.
Yes, you are certainly welcome to pull my PR and adjust the CloudFormation template. There are also other issues that are worth addressing, for example, parametrization of the ecr repository, certificate, aws account id, etc. I started the TODO section in the README. Please add anything you see fit there, as well as push further improvements.
This was meant as a PoC reference implementation. I would certainly be delighted to pull any upstream changes.
There was a problem hiding this comment.
Understood! I think the dockerfile bits is at least not something in line with our own standards, and it's only fair we tackle those. Another thing is dispatch, which isn't really enabled. Thanks for your contribution! As soon as we get some spare cycles we will get back to this 🙇🏻
There was a problem hiding this comment.
Yes, I do understand. You are correct. Appreciate the fact that this is a draft rather than a finished production-quality template, and it does require someone to co-pilot before the merge.
Perhaps in the future, I will get to the point where I will be able to cover an entire issue by myself, all the way over the finish line. Right now, I built MAYBE somewhat useful pieces. However, I don't have enough mileage to see every angle, like the core team members. Just sharing small bits that I developed while trying to get the initial SpiceDB integration functional enough to cover some of my use cases.
| AutoScalingTargetValue: | ||
| Type: Number | ||
| Default: 50 |
There was a problem hiding this comment.
autoscaling could be disruptive to a spicedb cluster, because the ring has to reconfigure and it would potentially affect your deployment's cache hit ratios
There was a problem hiding this comment.
could you please rephrase your comment as a question?
BTW, I do have a question here, since I know next to nothing about the project: Does SpiceDB support dynamic sizing/scaling or not?
There was a problem hiding this comment.
My initial suggestion was not to enable autoscaling. While SpiceDB supports adding and removing nodes, it's something that comes at a cost of reorganizing the hash ring, and leads to lower cache hit rate. However, after discussing with the team, there are things we could explore to reduce the impact of instances of SpiceDB coming and going, and we certainly want to make autoscaling fully supported without any performance impact.
So you can dismiss my comment!
No description provided.