-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test cache first then download (and don't fail when online doesn't work if it can be resolved to cache) #211
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,14 +77,20 @@ public File createCache(URI repositoryLocation, String prefix, IProgressMonitor | |
|
||
@Override | ||
public File createCacheFromFile(URI remoteFile, IProgressMonitor monitor) throws ProvisionException, IOException { | ||
File cacheFile = getCacheFile(remoteFile, getCacheDirectory()); | ||
if (offline) { | ||
File cacheFile = getCacheFile(remoteFile, getCacheDirectory()); | ||
if (cacheFile != null) { | ||
return cacheFile; | ||
} | ||
throw new ProvisionException(getFailureStatus(remoteFile)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the "throw" here be preserved in case of offline + cache miss? I have the impression modified code would try to download the file in that case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, not sure why i removed that, force pushed a new change |
||
} | ||
return super.createCacheFromFile(remoteFile, monitor); | ||
try { | ||
return super.createCacheFromFile(remoteFile, monitor); | ||
} catch (IOException e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be merged as |
||
return handleCreateCacheException(cacheFile, remoteFile, e); | ||
} catch (ProvisionException e) { | ||
return handleCreateCacheException(cacheFile, remoteFile, e); | ||
} | ||
} | ||
|
||
private Status getFailureStatus(URI uri) throws ProvisionException { | ||
|
@@ -100,8 +106,8 @@ public static File getCacheFile(URI url, File dataAreaFile) { | |
return new File(dataAreaFile, Integer.toString(hashCode)); | ||
} | ||
|
||
private <T extends Exception> File handleCreateCacheException(File cacheFile, URI repositoryLocation, T e) | ||
throws T { | ||
private File handleCreateCacheException(File cacheFile, URI repositoryLocation, Exception e) | ||
throws ProvisionException { | ||
if (cacheFile != null) { | ||
String message = "Failed to access p2 repository " + repositoryLocation.toASCIIString() | ||
+ ", use local cache."; | ||
|
@@ -114,7 +120,7 @@ private <T extends Exception> File handleCreateCacheException(File cacheFile, UR | |
// original exception has been already logged | ||
return cacheFile; | ||
} | ||
throw e; | ||
throw new ProvisionException(getFailureStatus(repositoryLocation)); | ||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the "IOException" be removed as possible "throws" ?