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

Add typing to _ConnectionParameters and related functions #1199

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

DanielNoord
Copy link
Contributor

This adds typing to an important NamedTuple in this module that will allow using types in a lot of other functions that have a reference to this object.

The final type looks very similar to what was suggested in #577 but this is based on the paths that the code can actually take. As with other PRs the strategy is simple: start with the lowest function in the call chain to see if the code can tell you what types it should accept and propagate upwards until you reach the NamedTuple.
The most controversial change is probably _ConnectionParameters.ssl. This is because of the code path where we set sslmode = SSLMode.disable. It is a bit annoying but that code path means that ssl never gets reassigned and therefore will be whatever _parse_connect_dsn_and_args accepts. We should probably change that to be None but I don't want to introduce behaviour changes in these PRs.

The two actual runtime changes are straightforward and any mistakes in the typing we can (of course) always rectify later.

@DanielNoord
Copy link
Contributor Author

Based on review in #1198 I have swapped out the stringified annotations for non-stringified behind a from __future__ import annotations.

I also had to rebase and force push due to merge conflicts.

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