fix: add DB keepalives #358
Conversation
| @@ -115,6 +118,9 @@ jobs: | |||
| sed -i ''s^#DB_USER#^$DB_USER^g'' ./deployment/deployment.yml | |||
| sed -i ''s^#DB_PASS#^$DB_PASS^g'' ./deployment/deployment.yml | |||
| sed -i ''s^#DB_NAME#^$DB_NAME^g'' ./deployment/deployment.yml | |||
| sed -i ''s^#DB_KEEPALIVE_IDLE#^$DB_KEEPALIVE_IDLE^g'' ./deployment/deployment.yml | |||
There was a problem hiding this comment.
Don't these strings needed to be added to deployment/deployment.yml too?
And can we add them as values not secureValues, I think that means we can actually see what they are in Azure? (as we won't be able to in GitHub)
There was a problem hiding this comment.
Yes, thanks, good spot.
It's possible to see any secret value sent to an Azure container instance by connecting to the container and echoing the value to the terminal, but I agree it would make sense to have these as values too, so that extra step is not needed.
It would also be good to have them in Github as variables, not secrets. What do you think? I think last time I suggested this you weren't sure because the existing non-secret values had been entered as secrets, but it would be good to start fixing this, especially as we'll have more opportunity to do so with all the upcoming work on the pipeline.
src/constants/config.py
Outdated
| DB_CONN_RETRY_LIMIT=8, | ||
| DB_CONN_SLEEP_START=5, | ||
| DB_CONN_SLEEP_MAX=60, | ||
| DB_CONN_TIMEOUT=5, | ||
| DB_KEEPALIVE_IDLE=os.getenv("DB_KEEPALIVE_IDLE"), |
There was a problem hiding this comment.
Could there be defaults here or in .env-example? Make it easier to set up local environments
There was a problem hiding this comment.
Will add in .env-example
simon-20
force-pushed
the
add_keepalives
branch
from
7c17900 to
d73f41a
Compare
Resolves #317