-
Notifications
You must be signed in to change notification settings - Fork 550
Add Cal.com API v2 service and Redis to Docker Compose #459
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
Conversation
Thanks for this and the extra detail. @dcharles525 @hadams95 please consider this in the work you're doing for the helm chart, and related to the API v2 container image reqs. https://github.com/calcom/cal.com/blob/main/apps/api/v2/Dockerfile Also more context: calcom/cal.com#21804 |
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.
The calcom api won't start properly without the VAPID keys set as environment variables, this should be added to the env.example and the configuration section of the README as well.
Otherwise looks good to me.
- Adds instructions on setting up VAPID keys for push notifications to the README. - Also adds VAPID keys, Stripe keys, and a team billing flag to the `.env.example` file.
Thank you for the feedback @dcharles525, I've added the requested missing elements to the |
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.
Thanks for the PR! LGTM!
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'd like to request making sure all env vars can pick up .env vars (if available). For example, API_KEY_PREFIX can be changed, but it's hardcoded in the docker-compose changes. IS_E2E doesn't match the exact .env example, but it could, via IS_E2E=${NEXT_PUBLIC_IS_E2E:-false}
- Allows configuration of Redis and API via environment variables. This change updates the docker-compose.yaml and .env.example files to allow the Cal.com application to be more easily configured via environment variables. This provides greater flexibility in deployment scenarios.
Thank you for the feedback @krumware, that actually makes sense and allows greater flexibility in configuring the API service indeed. I've made the requested updates see the latest the commit: 267a27b |
@percymamedy sorry to be a bother. This is a good chance for me to enforce parity with all of the .env.examples. (You'll know the env vars better than me, but my preferred goal would be to allow a user to reuse their calcom/cal.com .env's directly with the docker images with few changes) |
@krumware yes REDIS_URL is not in the As for the parity between the other env vars, I'll push a change and match them as close as possible, I did not know initially which convention to follow and saw |
@krumware I've made changes to ensure to match env vars as close as possible with the env vars in the api v2 |
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.
lgtm
thank you! |
@dcharles525 @hadams95 Please withhold merging until the apiv2 image is published |
Summary
This PR adds a new
calcom-api
service for running the Cal.com API v2 instance alongside the existing Cal.com web application, and introduces a Redis service for caching and session management.Changes Made
New Services Added
redis
service with persistent volume storage for caching and session managementcalcom-api
service running on port 80 (mapped to host port 80)Service Configuration
./calcom
context withapps/api/v2/Dockerfile
Environment Variables
REDIS_URL
pointing to the Redis serviceBuild Optimization
Benefits
Testing
Notes
.env
file