-
Notifications
You must be signed in to change notification settings - Fork 2.6k
skip ssl
import if not available
#3078
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
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #3078 +/- ##
==========================================
- Coverage 91.84% 91.83% -0.01%
==========================================
Files 128 128
Lines 33232 33283 +51
==========================================
+ Hits 30523 30567 +44
- Misses 2709 2716 +7 ☔ View full report in Codecov by Sentry. |
Hi @dicej, can you please update your code to follow the pattern set in https://github.com/redis/redis-py/blob/master/redis/client.py for ssl import and type hints and fix the conflicts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested changes are posted in previous comment.
Signed-off-by: Joel Dice <[email protected]>
Done, and tested on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dicej please fix the linter errors and apply requested changes
Signed-off-by: Joel Dice <[email protected]>
Signed-off-by: Joel Dice <[email protected]>
* skip `ssl` import if not available Signed-off-by: Joel Dice <[email protected]> * address review feedback and fix lint errors Signed-off-by: Joel Dice <[email protected]> * remove TYPE_CHECKING clause from ssl conditional Signed-off-by: Joel Dice <[email protected]> --------- Signed-off-by: Joel Dice <[email protected]> Co-authored-by: petyaslavova <[email protected]>
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
utils.py
has support for detecting whether thessl
module is available, and we can use this to omit SSL-specific funcionality while still providing other features (e.g. unencrypted connections).Prior to this patch, the
connection.py
modules both triggered anImportError
due to unconditional imports of thessl
module. Now, we checkutils.SSL_AVAILABLE
prior to attempting the import and only raise an error later if (and only if) the application requests an encrypted connection. This helps support platforms such aswasm32-wasi
where thessl
module is not built by default.Ideally, this would be tested in CI using a Python built without
ssl
support. I'm not sure how feasible that is, though; I'm open to guidance on that aspect.