fix(threading): Handle channels shadowing#5299
Conversation
|
|
||
| channels_version = channels.__version__ | ||
| except ImportError: | ||
| except (ImportError, AttributeError): |
There was a problem hiding this comment.
this implies that if there is an AttributeError because of channels line, django_version will be set to None and that is not ideal... perhaps we should split this try/except in 2 parts, 1 for django and another for channels?
|
|
||
| channels_version = channels.__version__ | ||
| except ImportError: | ||
| except (ImportError, AttributeError): |
There was a problem hiding this comment.
this implies that if there is an AttributeError because of channels line, django_version will be set to None and that is not ideal... perhaps we should split this try/except in 2 parts, 1 for django and another for channels?
There was a problem hiding this comment.
That's ok, django_version and channels_version both are only used for warning about a very specific Django/channels/sentry-sdk incompatibility that only appears in specific versions.
But you raise a good point in that it's not clear what the variable is used for. It might happen that someone extends the logic at some point using django_version for some other purpose, not realizing it's not always populated correctly. So it's worth making this nicer.
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Documentation 📚
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Description
Folks might have a
channelsmodule that has nothing to do with thechannelspackage we're expecting. This will then make the code throw anAttributeErrorwhich we don't handle.Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)