Skip to content

Add option to update TLDs list over proxy#150

Closed
czechnology wants to merge 2 commits intojohn-kurkowski:masterfrom
czechnology:master
Closed

Add option to update TLDs list over proxy#150
czechnology wants to merge 2 commits intojohn-kurkowski:masterfrom
czechnology:master

Conversation

@czechnology
Copy link

I am in a scenario where the Internet can only be accessed over a proxy, yet that proxy is not meant to be in env. vars.
This PR adds option to update the TLDs over a proxy, without unnecessarily cluttering the main class initializer with proxies.

@floer32
Copy link
Collaborator

floer32 commented Mar 4, 2019

First reaction was - hmm I wish we did not have to cascade this argument through a number of places. But I do agree it's ideal not to have it in the constructor. Just wondering if at this point, should it support passing in a Session instance instead of this one argument?

@john-kurkowski thoughts?

@john-kurkowski
Copy link
Owner

I agree @hangtwenty. Let's not maintain our own requests shim language, always playing catchup. Let the caller optionally decide how we should make requests, with whatever requests options they want.

Copy link
Collaborator

@floer32 floer32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@czechnology Support the overall idea but if we are opening it up to customization let's really open it up. Could you change it to support customization by passing Session? this will allow the proxy use case (but probably won't even mention the proxy argument specifically in the code now, as it's a requests detail, not us).

ignore snippet I just deleted, I meant to delete that before posting (realized that part was wrong)

@floer32 floer32 added the see issue #158 [one fell swoop] fixed when we fix https://github.com/john-kurkowski/tldextract/issues/158 - temporary label. label Mar 5, 2019
@john-kurkowski
Copy link
Owner

Closing due to inactivity. Concentrate on #158.

@john-kurkowski john-kurkowski mentioned this pull request Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hmm 🤔 see issue #158 [one fell swoop] fixed when we fix https://github.com/john-kurkowski/tldextract/issues/158 - temporary label.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants