-
Notifications
You must be signed in to change notification settings - Fork 405
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
feat: Support Aptible deployments #4340
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Docker builds report
|
Uffizzi Preview |
23d9f2b
to
b3cd868
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4340 +/- ##
==========================================
+ Coverage 96.77% 96.84% +0.06%
==========================================
Files 1159 1166 +7
Lines 38069 38675 +606
==========================================
+ Hits 36842 37453 +611
+ Misses 1227 1222 -5 ☔ View full report in Codecov by Sentry. |
Got feedback from the prospect that this is working for them. The only thing that I haven't got working yet on Aptible is the task processor. It is possible to run it but need to keep experimenting with different settings. |
Should we mark this PR as draft until this is done then? Or merge this and submit another PR for the task processor? |
I vote to merge and then additional PRs if there's a need |
Thanks for submitting a PR! Please check the boxes below:
pre-commit
to check lintingdocs/
if required so people know about the feature!Changes
This PR adds functionality to enable deploying to Aptible, which an Enterprise prospect is currently evaluating. They have been able to work around these changes by forking open-source Flagsmith and making similar changes (i.e. removing
waitfordb
instead of making it optional). They would however need the changes to be upstreamed to deploy the Flagsmith Enterprise image.Add
/healthcheck
as a health check routeExposes the same health check endpoint on
/healthcheck
in addition to/health
. Aptible (surprisingly) does not allow configuring a custom health check route and forces it to be/healthcheck
. Strict health checks also do not accept redirects as responses.There are some alternatives to directly adding
/healthcheck
as an alias of/health
:/healthcheck
route, using some sort of "Aptible mode". I don't see a downside to always aliasing/health
so this seems like extra complexity for no benefit./health
. I believe YAGNI also applies here and we can add it later if needed.Add
SKIP_WAIT_FOR_DB
environment variableWhen Aptible is deploying an application, it first checks for successful health checks before it is added to the same network as an Aptible-hosted database. This means that trying to deploy Flagsmith to Aptible without these changes will always fail when health checks fail while
waitfordb
is hanging.This PR adds an environment variable that skips waiting for the database before running migrations or starting the API.
It seemed to me that an environment variable is the least intrusive and Aptible-specific way we could support this - happy to consider different approaches and variable/parameter names.
How did you test this code?
Deployed Flagsmith to a trial Aptible account here using the steps described in the docs from this PR: https://app-76164.on-aptible.com/. I'll add a link in the docs to the actual Flagsmith release that contains these changes once ready.
Please describe.