feat: add option to overwrite Docker read and open timeout values

[yeikel]

What are you trying to accomplish?

Allows overwriting the read and open timeout values for interactions with the docker registry using two separate environment variables

What issues does this affect or fix?

I do not have any issue in the tracker, but I noticed this need while using dependabot-core with slow private registries

If there were multiple ways to approach the problem, why did you pick this one?

Initially, I considered using the existing environment variables defined here :

dependabot-core/common/lib/dependabot/clients/github_with_retries.rb

Lines 13 to 14 in c95a0fc

	DEFAULT_OPEN_TIMEOUT_IN_SECONDS = 2
	DEFAULT_READ_TIMEOUT_IN_SECONDS = 5

But I noticed that the default values there are different and I figured that separate environment properties may be better

Checklist

• I have run the complete test suite to ensure all tests and linters pass.
• I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
• I have written clear and descriptive commit messages.
• I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
• I have ensured that the code is well-documented and easy to understand.

[yeikel]

The e2e failures seem unrelated to my change

[yeikel]

@jeffwidman Any chance you can review this?

[sachin-sandhu]

Hi @yeikel , Thanks for the contribution!! We are reviewing the changes.

[sachin-sandhu]

Hi @yeikel , do you happen to know the default value for open_timeout if no value to supplied to DockerRegistry2, is it 2 seconds?

DockerRegistry2::Registry.new(
"https://#{registry_hostname}",
user: registry_credentials&.fetch("username", nil),
password: registry_credentials&.fetch("password", nil),
read_timeout: :read_timeout_in_seconds,
open_timeout: ??,
http_options: { proxy: ENV.fetch("HTTPS_PROXY", nil) }
)

[sachin-sandhu]

approving the changes.

[sachin-sandhu]

Hi @yeikel , i suspected the smoke test fails are due to your changes, so i created a new PR with your changes and it started failing #11425 , can you please assess your changes once what might cause smoke tests to fail, Thanks !

[yeikel]

Hi @yeikel , do you happen to know the default value for open_timeout if no value to supplied to DockerRegistry2, is it 2 seconds?

DockerRegistry2::Registry.new( "https://#{registry_hostname}", user: registry_credentials&.fetch("username", nil), password: registry_credentials&.fetch("password", nil), read_timeout: :read_timeout_in_seconds, open_timeout: ??, http_options: { proxy: ENV.fetch("HTTPS_PROXY", nil) } )

Hi @sachin-sandhu See the defaults here https://github.com/deitch/docker_registry2/blob/bfde04144f0b7fd63c156a1aca83efe19ee78ffd/lib/registry/registry.rb#L26-L27

[yeikel]

Hi @yeikel , i suspected the smoke test fails are due to your changes, so i created a new PR with your changes and it started failing #11425 , can you please assess your changes once what might cause smoke tests to fail, Thanks !

You are right. I updated it and it should be ready now.

I also documented the default values

[sachin-sandhu]

Approving based on discussions and changes.

[sachin-sandhu]

@yeikel , thanks for the changes , the test cases are good now , i have approved it and will be deploying/merging today. Thanks for improving !!

[sachin-sandhu]

Update: PR was deployed and merged, we didn't find any issues while monitoring

cc @yeikel , @abdulapopoola