Use cached remote config on network failure #4709
Conversation
lib/rubocop/remote_config.rb
Outdated
| @@ -40,11 +43,13 @@ def request(uri = @uri, limit = 10, &block) | |||
| end | |||
|
|
|||
| handle_response(http.request(request), limit, &block) | |||
| rescue SocketError | |||
| handle_response(NetworkInaccessible.new, limit, &block) | |||
There was a problem hiding this comment.
Do you really need the new NetworkInaccessible class, or could you just use the SocketError object?
There was a problem hiding this comment.
Nah, it's not necessary. It provides some abstraction if there are more cases to deal with than just SocketError, but I haven't seen others yet.
lib/rubocop/remote_config.rb
Outdated
| end | ||
|
|
||
| def handle_response(response, limit, &block) | ||
| case response | ||
| when Net::HTTPSuccess, Net::HTTPNotModified | ||
| when Net::HTTPSuccess, Net::HTTPNotModified, NetworkInaccessible |
There was a problem hiding this comment.
I wasn't totally happy with this list containing both HTTPResponses and *Errors, but it was the simplest path. Any other ideas?
After the rescue on L46, we do have the option of just doing nothing, but I like handle_response being applied consistently.
|
Great addition! The failing spec looks like an intermittent one. Likely concurrency problem because of parallel tests. No need to worry about that one. |
|
@Drenmi Ah that's a relief, thanks :) I figured it was unrelated, but didn't have an explanation. |
|
Removed |
If your configuration provides a URI to `inherit_from`, the locally
cached file is stale, and you are offline, then running RuboCop would
crash with an error like:
Failed to open TCP connection to example.com:443 (getaddrinfo:
nodename nor servname provided, or not known)
Since we do have a cached config, we can use it, and you can still run
cops without having to wait for the network to be available again.
Effectively, this behaves the same as if the network were accessible,
but the configuration file has not been modified.
If the file has never once been cached, the result is a different crash
reporting that the cached file does not exist. This may be confusing and
could be handled better, but is no worse than RuboCop's behavior when
you specify a nonexistent configuration file.
kristjan
force-pushed
the
offline_network_config
branch
from
1e890dd to
2d686a5
Compare
|
Nice improvement! 👍 |
If your configuration provides a URI to
inherit_from, the locallycached file is stale, and you are offline, then running RuboCop would
crash with an error like:
Since we do have a cached config, we can use it, and you can still run
cops without having to wait for the network to be available again.
Effectively, this behaves the same as if the network were accessible,
but the configuration file has not been modified.
If the file has never once been cached, the result is a different crash
reporting that the cached file does not exist. This may be confusing and
could be handled better, but is no worse than RuboCop's behavior when
you specify a nonexistent configuration file.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).rake spec) are passing.rake internal_investigation.and description in grammatically correct, complete sentences.
rake generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).