Add option to override the inherited HTTP client configuration

@Claudiu Cristea has a wonderful module https://www.drupal.org/project/http_request_mock → that makes it easy to create HTTP service mocks, but unfortunately I wasn't able figure it out how can I test / mock timeouts... I basically spent my yesterday afternoon trying to do that, but nothing I've tried has led to success, so I'm just testing the configuration here.

To be fully BC, the new configuration is declared as nullable, so no preexisting ImagecacheExternal installation will have any change in their module settings if this FR is merged and released.

Test came back green, unassigning.

This is working as described but I would like to challenge the BC approach. In the MR, you are optionally saving the value in the config yaml file, in order to maintain BC. You are also providing a test that saving the settings will omit the value if it is the default (to test BC).
Why not simply provide an update path which update the config file to include the default value as a standard way of storing it? that way, you do not even need tests to verify the changes, and the projects adapting to it, can use tests that cover hook updates. I don't think that BC means you cannot change stuff. It only means that existing projects should work as they already do.
If there is a reason for that, then it is RTBC from me (apart from one typo nitpick).

To be honest, on the one hand, I didn't want to think about what the value should be in the case when we want to inherit the default configuration. On the other hand, because of DX: as the maintainer of a Drupal site, when I update a module, I prefer no to see any changes in a module configuration if it works like it did before (mainly because it reduces the noise I see in the commits I make if I upgrade a module).

Attaching patch on top of the MR of ✨ SVG support RTBC

I prefer no to see any changes in a module configuration if it works like it did before

It is a fair explanation for me. I will leave it to the maintainer. I would still prefer the other way, but I don't have a reason to insist :) +1 RTBC

Great work here - you might be able to use https://www.previousnext.com.au/blog/testing-code-makes-http-requests-dr... to simulate a timeout

We will need an update here, the config object should always contain all the valid keys, even if they're empty - so that config validation can occur.

@huzooka if you're using this module at work, I'd be happy to take on a co-maintainer - thanks for all the issues. If so, please file a new issue with offer to co-maintain. I don't use it anymore, but in what is a great turn of fate, I originally wrote it for the Greek government on D6 to use on a tourism website, so returning to the EU government feels like a nice end result.

We recently added @swentel who merged a lot of issues and moved things forward. But another set of hands always welcome.

Re #10

1. I spent some time trying to mock a timeout but unfortunately it didn't work (with my current knowledge I don't think it will as long as the test and tested Drupal instance are running in the same PHP thread).

2. the config object should always contain all the valid keys, even if they're empty - so that config validation can occur.

   If the configuration key(s) are nullable, then they do not always have to be included in the config object. You verify this if you add this to the settings form test (we cannot trigger schema violations if we use the config form through the test browser):

   // These do not cause config schema violation as long as the
   // "http_client_config" "http_client_config.timeout]" keys are defined as being
   // nullable.
   $config_factory->getEditable('imagecache_external.settings')
     ->set('http_client_config', [])
     ->save();
   $config_factory->getEditable('imagecache_external.settings')
     ->set('http_client_config', ['timeout' => 10])
     ->save();
   $config_factory->getEditable('imagecache_external.settings')
     ->set('http_client_config', ['timeout' => '20'])
     ->save();
   $config_factory->getEditable('imagecache_external.settings')
     ->clear('http_client_config')
     ->save();
   
   // ...But any of these do!
   $config_factory->getEditable('imagecache_external.settings')
     ->set('http_client_config', ['missing_key' => 'value'])
     ->save();
   $config_factory->getEditable('imagecache_external.settings')
     ->set('http_client_config', ['timeout' => 'string'])
     ->save();
   $config_factory->getEditable('imagecache_external.settings')
     ->set('http_client_config', 'string')
     ->save();
   

3. Thank you very much for the honor, but I don't live with it. (I am terribly behind in the maintenance of my other modules, unfortunately 🫣.)