(#2195) Added timeout to registry download call

[Raalsky]

resolves #2195 (potentially)

Description

Added a 10 seconds timeout to every download call during dbt deps. I think the timeout is only for waiting until the response will be started to emmiting (the download process may be longer than timeout). See requests - Timeouts for more info.

You can tell Requests to stop waiting for a response after a given number of seconds with the timeout parameter. Nearly all production code should use this parameter in nearly all requests. Failure to do so can cause your program to hang indefinitely:

Checklist

• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

[Raalsky]

I have run this code in development and it appears to resolve the stated issue

The testing part looks awful I think (mock the requests internals or create /etc/hosts with desired low connection service). And as it looks to be a general good practice to add a timeout to requests and we actually don't know the 1:1 reason why the connection hangs maybe we don't need to add unit/integration test to these parts? 🤞

[beckjake]

This looks good to me! I think there's not much value in testing this relative to the difficulty of making a test where the network hangs forever.

In the future, we may want to make this timeout configurable, even if it's just via an environment variable. While 10s is a totally reasonable maximum value for most places, it might not be great if you are both far away and have a bad connection. We'll see how this shakes out for now.

I'll kick off the remaining tests, this is approved assuming they pass.

[Raalsky]

@beckjake good point! I was thinking about using a global config or something which should be used here but (maybe I'm wrong) there is nothing available in registry.py throw self or something and I need to add a lot of refactors. Do we have some policy here (for ex. static CONNECTION_TIMEOUT at beginning of file etc.)?

[beckjake]

Do we have some policy here (for ex. static CONNECTION_TIMEOUT at beginning of file etc.)?

Global constants are always nice, though I felt this was unambiguous enough to just leave it. I agree it'd be quite tough to get it through from a config file given the current state of registry.py, and it's why I thought of an environment variable.

[drewbanin]

I'd be comfortable with adding a DBT_HTTP_TIMEOUT env var which we use whenever we download data with requests. I just want to make sure it's clear that this controls http web requests, not database connections, as those timeouts are going to be handled separately.

For the purposes of this PR, it would be great if we could apply this env var override in the scope of downloading data from the package hub

[Raalsky]

In the future, we may want to make this timeout configurable, even if it's just via an environment variable. While 10s is a totally reasonable maximum value for most places, it might not be great if you are both far away and have a bad connection. We'll see how this shakes out for now.

I'd be comfortable with adding a DBT_HTTP_TIMEOUT env var which we use whenever we download data with requests. I just want to make sure it's clear that this controls http web requests, not database connections, as those timeouts are going to be handled separately.

Done

[Raalsky]

I'll move the environ variable usage inside system.py insted of registry.py.