Privacy plugin for DEFAULT_TIMEOUT_IN_SECS

[toomore]

Before raising an issue, initiate a discussion to gather more feedback and suggestions.

I encountered a timeout error when fetching https://cct.google/taggy/agent.js on GitHub Actions.

WARNING - Couldn't retrieve https://cct.google/taggy/agent.js: HTTPSConnectionPool(host='cct.google', port=443): Read timed out. (read timeout=5)

I noticed that the Privacy plugin has DEFAULT_TIMEOUT_IN_SECS in the global variables.

mkdocs-material/src/plugins/privacy/plugin.py

Line 47 in 252167d

DEFAULT_TIMEOUT_IN_SECS = 5

I would like to request the addition of a new configuration variable for this plugin.

plugins:
  - privacy:
      timeout_in_secs: 5

[squidfunk]

We're in feature freeze right now due to foundational work, but you can raise a feature request if you like.

[kamilkrzyskow]

This is an unforeseen regression of:

• Fix #8012 - Privacy plugin crashes on HTTP errors #8015

Without a specified timeout, according to the docs, requests doesn't set a timeout, so it could take minutes to download:

• https://docs.python-requests.org/en/latest/user/advanced/#timeouts

The issue described in #8012 likely needed the response.raise_for_status() but not the timeout, but in the logs there was a TimeoutError already hmm 🤔

I do think that a timeout is beneficial, so we could also be more generous with the given time and use the split for connect and read times, with multiples of 3: (6.1, 12)
What do you think @Lucas-C ?

[squidfunk]

So, as I read it: rather set no timeout as a default, but allow to override it with a specific value. In most cases it might make sense to set a timeout though, so maybe we should allow to null it, with a default of a 30s or so. The reason is that I don't want the CI I pay for to hang because of a server responding too slowly. However, IMHO, splitting connect and read sounds overcomplicated.

[Lucas-C]

The reason is that I don't want the CI I pay for to hang because of a server responding too slowly. However, IMHO, splitting connect and read sounds overcomplicated.

I agree.

If you do not set a timeout, the HTTP call may never complete, which will slow down mkdocs...

It's also a common best practice for HTTP clients to set a timeout, which ensure your request will not take more than N seconds.
If a request takes more than 5min to complete, the problems is probably not with the Python code, but with the server serving the HTTP answer...

Now, maybe 5 seconds is too low, and 30s or more would be a better default value, but IMHO setting no timeout isn't a good idea.

[kamilkrzyskow]

I wouldn't want to wait 30 seconds for the server not to respond at all, but I guess that connect issues are quite rare 🤔

To be clear I thought about setting DEFAULT_TIMEOUT_IN_SECS to a (6.1, 12) or now (6.1, 30), and later once modification of the configuration is unfrozen, add a single timeout_in_secs config option that would be a custom type with custom validation, allowing the user to decide what to pass to the requests.get function, an int or list[int]. Validation could also convert the list to tuple if that's what requests requires.

There is no need for 2 separate settings for timeout_connect and timeout_read which would add the complexity for general users, but I don't see a reason why not to support both input variants.

[squidfunk]

The is no need for 2 separate settings for timeout_connect and timeout_read which would add the complexity for general users, but I don't see a reason why not to support both input variants.

I'm not sure whether tuples are well supported in YAML/MkDocs configuration syntax. Also, from my experience, MkDocs doesn't really like unions of non-primitives. Thus, I don't think we will support multiple variants.

[kamilkrzyskow]

Hi @toomore,
apart of the comment response above, I also can suggest to use some monkey patching to modify the global yourself before the _fetch functions executes, this can be done in a proxy script or hook:

• Group plugin breaks if environmen value is "1", must be "true" to work #7976 (comment)
• https://www.mkdocs.org/user-guide/configuration/#hooks