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

Remove global register_schemes calls #246

Closed

Conversation

jor-rit
Copy link

@jor-rit jor-rit commented Jan 13, 2020

Modifying the global urllib.parse/urlparse uses_* variables isn't needed anymore. Most aren't used and are only left for backwards_compatibility.
Only one that could be relevant would be uses_param (for parsing path variables). But theses aren't used/read from the result anywhere in django-environ.

Modifying the global urllib.parse/urlparse uses_* variables isn't needed anymore. Most aren't used and are only left for backwards_compatibility.
Only one that could be relevant would be `uses_param` (for parsing path variables). But theses aren't used/read from the result anywhere in django-environ.
@jor-rit
Copy link
Author

jor-rit commented Jan 13, 2020

Ran into this while extending Env to add support for extra url schema I needed.
Caused a bit of confusion (e.g. should/can we call this to add extra schemes),
so looked into what it's actually doing, specifically with these "undocumented, but have a public-looking name" uses_* variables in urllib.parse/urlparse.

As far as I can see this isn't needed anymore, none of the db/email/cache/search URL actually read the 'params' part of urlparse result...
And uses_query and uses_fragment aren't used in urllib anymore (so these are always parsed), only left for backwards compatibility.

@sergeyklay sergeyklay deleted the branch joke2k:develop September 4, 2021 12:00
@sergeyklay sergeyklay closed this Sep 4, 2021
@sergeyklay sergeyklay reopened this Sep 4, 2021
@coveralls
Copy link

coveralls commented Sep 4, 2021

Coverage Status

Coverage increased (+0.4%) to 90.98% when pulling 170880c on jor-rit:remove-global-register-schemes into cbbd6d6 on joke2k:develop.

@sergeyklay
Copy link
Collaborator

PR was closed by mistake. Reopened it.

@sergeyklay sergeyklay deleted the branch joke2k:develop October 19, 2021 21:45
@sergeyklay sergeyklay closed this Oct 19, 2021
@sergeyklay sergeyklay reopened this Oct 20, 2021
@sergeyklay sergeyklay added the tech debt Technical debt issues - which point out issues we should resolve long-term label Jun 16, 2022
@sergeyklay sergeyklay self-assigned this Jun 16, 2022
@sergeyklay
Copy link
Collaborator

Implementing in #399. Thank you for the patch, and for helping make django-environ better! And I'm sorry for the delay.

@sergeyklay sergeyklay closed this Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Technical debt issues - which point out issues we should resolve long-term
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants