bblimke
Suggestion: Explicit disallow option for .allow_net_connect!
Currently, in the code, .disable_net_connect allows users to specify explicitly allowed connections. However, the opposite is not true. If I wanted to .allow_net_connect! on everything, but disallow specifics ones, there is currently no way to do this using the .allow_net_connect! method, which seems unintuitive to me .
The specific example use case that I'm thinking of, would be using Webmock in a development environment, where you want to allow all API calls by default, but perhaps disable some specific costly/annoying ones. I don't know which APIs I might add in the future, so I don't want to have to disallow all by default, because then I will have to configure Webmock in development every time.
What do you think? I'm a pretty new developer, so there's a strong possibility that I'm missing something, or that this feature is already covered somehow, so please let me know if this is a wrongheaded idea.
def self.allow_net_connect!(options = {})
Config.instance.allow_net_connect = true
Config.instance.net_http_connect_on_start = options[:net_http_connect_on_start]
#New Code:
Config.instance.disallow options[:disallow]
end
to better match:
def self.disable_net_connect!(options = {})
Config.instance.allow_net_connect = false
Config.instance.allow_localhost = options[:allow_localhost]
Config.instance.allow = options[:allow]
Config.instance.net_http_connect_on_start = options[:net_http_connect_on_start]
end
Oof, I just realized you have a specific mailing list for suggestions, I will close this issue and ask there. My apologies
Actually, the mailing list seems pretty dead, and maybe broken (I started a conversation, but do not see it). Is this the best place to propose suggestions?
If the maintainers think this is a good idea, I may try to take this on myself, but I wanted to be sure it would be helpful before taking it on.
@sandfortw Thank you for suggesting that feature. I agree it makes sense to add it.
I'm not sure about the disallow and allow naming, since I feel except would be more readable. Similarly, I think disable_net_connect could be replaced with something more intuitive, like disable_real_connections or disable_real_requests.
However, for consistency with the existing API, I'm inclined to stick with disallow as you suggested.
If you're interested in submitting a pull request for this feature, I would greatly appreciate it.
