You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thanks for wanting to report an issue you've found in valkey-py. Please delete this text and fill in the template below.
It is of course not always possible to reduce your code to a small test case, but it's highly appreciated to have as much data as possible. Thank you!
Version: 5.1.0b7
Platform: Python 3.12 on MacOS ARM running OS Sonoma
Description:
In #23 I proposed including redis and rediss as valid protocols. I noticed while implementing the change that the code was almost identical with the main difference being variable names.
I propose that the protocol and url code be moved into a module that can be called in both places. This would reduce identical code reducing the amount of code needed to cover as well as making the testing for that portion of the code-base completely non-reliant on server setup and connection. This tests that the code does what it's supposed to do not that the code interacts with the platform how we expect it to (which should be a different and separate test in itself)
Proposed changes:
move parse_url to a new python module (or possibly refactor the aysnc connection to inherit from the synchronous one if possible)
create a dedicated test path for that parsing and move the identical tests in the sync/async versions of test_connection_pool.py.TestConnectionPoolURLParsing to that dedicated test module.
The text was updated successfully, but these errors were encountered:
kjaymiller
changed the title
single connection base object validation to reduce code/test complexity around connection URL Parsing
implement a single validation function around the service_uri to reduce code/test complexity around connection URL Parsing
Jun 27, 2024
It definitely makes sense to share parse_url implementation across the 2 classes. And we should keep the "asyncio" version, because the other is actually broken (it checks the URI scheme case sensitively which is against RFC3986).
It would certainly make sense to test the "new" parse_url in a single place, but I would still keep the separate TestConnectionPoolURLParsing (at least part of them) because they don't just test parse_url.
Thanks for wanting to report an issue you've found in valkey-py. Please delete this text and fill in the template below.
It is of course not always possible to reduce your code to a small test case, but it's highly appreciated to have as much data as possible. Thank you!
Version: 5.1.0b7
Platform: Python 3.12 on MacOS ARM running OS Sonoma
Description:
In #23 I proposed including
redis
andrediss
as valid protocols. I noticed while implementing the change that the code was almost identical with the main difference being variable names.I propose that the protocol and url code be moved into a module that can be called in both places. This would reduce identical code reducing the amount of code needed to cover as well as making the testing for that portion of the code-base completely non-reliant on server setup and connection. This tests that the code does what it's supposed to do not that the code interacts with the platform how we expect it to (which should be a different and separate test in itself)
Proposed changes:
test_connection_pool.py.TestConnectionPoolURLParsing
to that dedicated test module.The text was updated successfully, but these errors were encountered: