-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add keepalives by default to redshift db connection url #173
Conversation
These seem to prevent issues with long queries timing out. I couldn't find any great documentation about this, only this stackoverload thread and my own experimentation. https://stackoverflow.com/q/43594310/18421950
@austinweisgrau sorry it took me so long to review but seems like a very reasonable change; record mover is not building currently (see https://circleci.com/gh/bluelabsio/records-mover/16903 ) so we'll have to correct issue with nose-progressive not installing to be able to push this change out to people through Alectrona. |
Seems like @archetypalsxe (Jack) was working on this before he left: |
Here's some AWS info for reference: https://docs.aws.amazon.com/redshift/latest/mgmt/connecting-firewall-guidance.html. Pretty sure that the default is 5 mins so anything under that should avoid the issue. |
👋 I'm still consulting with BlueLabs! I might be looking back into this soon, there was just a shift of priorities and I ended up working on a bunch of other stuff. The biggest thing preventing us from upgrading to Airflow 2 is the build failing and trying to get that going again |
Shifting priorities? I don't believe it =p Is there anything we can help on with the breaking build that you're trying to fix with #171 ? Happy to scrub in if it'd be helpful. |
All tests passing, going to merge |
These seem to prevent issues with long queries timing out. I couldn't find any great documentation about this, only this stackoverload thread and my own experimentation.
https://stackoverflow.com/q/43594310/18421950