Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Do not check connection if local request #9968

Merged
merged 1 commit into from
Sep 14, 2017
Merged

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Sep 12, 2017

Follow up from external PR in #9862 to allow CI to execute.

cc @rjuszczyk

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Sep 12, 2017
@tobrun tobrun added this to the android-v5.1.4 milestone Sep 12, 2017
@tobrun tobrun self-assigned this Sep 12, 2017
Requests to servers which run on localhost should be independent from internet connection.
@@ -56,13 +56,14 @@ private HTTPRequest(long nativePtr, String resourceUrl, String etag, String modi
mNativePtr = nativePtr;

try {
// Don't try a request if we aren't connected
if (!Mapbox.isConnected()) {
HttpUrl httpUrl = HttpUrl.parse(resourceUrl);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmrm this code wasn't in the original PR, why was it introduced?

Copy link
Member Author

@tobrun tobrun Sep 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I triple checked it and the code in

try {
HttpUrl httpUrl = HttpUrl.parse(resourceUrl);
final String host = httpUrl.host().toLowerCase(MapboxConstants.MAPBOX_LOCALE);
// Don't try a request to remote server if we aren't connected
if (!Mapbox.isConnected() && !host.equals("127.0.0.1") && !host.equals("localhost")) {
throw new NoRouteToHostException("No Internet connection available.");
}
vs

https://github.com/rjuszczyk/mapbox-gl-native/blob/b113257bf5cf942029edf612313560e0a1c9bf04/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java#L58-L65

is the same except for formatting ( I rebased and fixed-up my formatting commit).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobrun Thanks for checking, it must be a diff artifact.

@tobrun tobrun merged commit 5e63e57 into master Sep 14, 2017
@tobrun tobrun deleted the external-pr-rjuszczyk branch September 14, 2017 05:45
@tobrun tobrun mentioned this pull request Sep 26, 2017
19 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants