Skip to content
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

WIP: Conn max age #139

Closed
wants to merge 4 commits into from
Closed

WIP: Conn max age #139

wants to merge 4 commits into from

Conversation

kingdonb
Copy link
Member

Replaces #75, now that #137 is merged this PR is just focused on Conn Max Age setting.

I'm still not understanding what failed or why there are two settings for CONN_MAX_AGE, but we can close #75 and continue the discussion here if needed. (Or go ahead and merge once tested 👍 )

I will squash out the WIP commits before merge

Kingdon Barrett added 3 commits November 27, 2020 08:25
teamhephy#75 (review)

>  I think this is a different CONN_MAX_AGE env variable?

I don't see where else this variable is used, but no other change is
present which could have caused the tests to fail ?
@kingdonb
Copy link
Member Author

I looked at this again in some more detail, tried to understand how CONN_MAX_AGE is used, tried to understand how Django imports the production DATABASE settings, I am not really a Django person and this looks like magic I don't know or understand. I'm also not sure why we wanted to set CONN_MAX_AGE, I think this change belonged to someone else.

I'm going to close it, but feel free to reopen if there is any need to set CONN_MAX_AGE. I'm not prepared to shepherd this change into the next release myself, particularly not being aware of the 'Why' makes it difficult to do testing effectively.

@kingdonb kingdonb closed this Nov 27, 2020
Maybe this needs to be translated into an int
@kingdonb kingdonb reopened this Nov 27, 2020
@kingdonb
Copy link
Member Author

I really don't know but saw this failed and wanted to try translating from string parameters into an int,

Maybe needs to be translated into a float #75 (comment), but hopefully floats and ints can get along with the + operator... let's just see if this fixes the build, then we can close (until someone comes along with a reason for needing to set CONN_MAX_AGE)

@kingdonb
Copy link
Member Author

So, that just failed in a different way. Closing until someone needs CONN_MAX_AGE, unless someone else know how to fix this easily.

@kingdonb kingdonb closed this Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant