Repository navigation
fix: make is_dsn_multihost agree with the connection plan - #64
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #36.
is_dsn_multihostandbuild_connection_planeach decided what "multi-host" means. They used different checks, and they disagreed:is_dsn_multihostbeforebuild_connection_planhost=a:5432&host=b:5433host=a,b&port=5432,5433host=a&host=b(no ports)ArgumentErrorArgumentErrorhost=a,a:5432/db, no hostNow there is one definition.
parse_connect_args(url)runs the asyncpg dialect's parse once and returns the connect args together with the(host, port)pairs to fail over to.build_connection_planandis_dsn_multihostboth use it.is_dsn_multihostis True exactly when the plan would have failover.The PR also stops documenting the portless multi-host DSN, which the connection factory rejects. That shape was in the
build_db_dsndocstring and the test fixtures. They now usehost=h1:p1&host=h2:p2.Release note
Behaviour change in
is_dsn_multihost:?host=h1,h2&port=p1,p2.sqlalchemy.exc.ArgumentErrorfor DSNs the connection factory would reject, for example repeatedhostparams without ports. It used to return True for these.Testing
main: a parametrised check thatis_dsn_multihostagrees withbuild_connection_planfor each DSN shape, the comma format, and theArgumentErrorcase.ruff,tyandeof-fixerare clean.