tldextract icon indicating copy to clipboard operation
tldextract copied to clipboard

[low urgency] [docs] add note about cache timeout argument

Open floer32 opened this issue 7 years ago • 2 comments

from PR #154 (unmerged), changing that to a docs ticket

Would be good to add a note to the README though since it does not point out this environment variable being available. [ TLDEXTRACT_CACHE_TIMEOUT is the current name]

it's undocumented currently, that you can set a timeout on the PSL-fetch request timeout.

could be added to README. but want the variable renamed...

floer32 avatar Mar 05 '19 19:03 floer32

variable rename notes

one wart: this variable has a name that could be misinterpreted ... it's not a 'cache timeout' in the sense of 'cache expiration', it's the timeout on the request which fetches the PSL -- which is then cached. (i think that's my fault! from a change a few years ago? 😅)

i don't think we should remove this variable name, at least without the normal DeprecationWarning process and then a major version upgrade eventually. however that seems like a yak shave.

could change it to os.environ.get('TLDEXTRACT_DEFAULT_FETCH_TIMEOUT, os.environ.get('TLDEXTRACT_CACHE_TIMEOUT')) so it uses a new better name, and document just that new name; with the old one silently supported as a legacy thing.

floer32 avatar Mar 05 '19 19:03 floer32

is this another "don't do anything, just do #158?"

timeout is something that can be controlled by requests.Session customization i.e. #158. so should we dismiss this ticket?

well, timeout is such a common need, and so simple to specify from an environment variable, that I get why we might retain the environment variable. with the rename noted above.

but it does mean that it's a very low priority issue.

PRs welcome anyways, to rename the variable, and add a little note to the docs.

floer32 avatar Mar 05 '19 19:03 floer32