[ 
https://issues.apache.org/jira/browse/MBUILDCACHE-32?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17751421#comment-17751421
 ] 

ASF GitHub Bot commented on MBUILDCACHE-32:
-------------------------------------------

olamy commented on code in PR #33:
URL: 
https://github.com/apache/maven-build-cache-extension/pull/33#discussion_r1285288318


##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -144,19 +145,32 @@ public void saveArtifactFile(CacheResult cacheResult, 
org.apache.maven.artifact.
      * @return null or content
      */
     @Nonnull
-    public Optional<byte[]> getResourceContent(String url) throws IOException {
+    public Optional<byte[]> getResourceContent(String url) {
+        String fullUrl = getFullUrl(url);
         try {
-            LOGGER.info("Downloading {}", getFullUrl(url));
+            LOGGER.info("Downloading {}", fullUrl);
             GetTask task = new GetTask(new URI(url));
             transporter.get(task);
             return Optional.of(task.getDataBytes());
-        } catch (Exception e) {
-            LOGGER.info("Cannot download {}", getFullUrl(url), e);
+        } catch (ResourceDoesNotExistException e) {
+            if (LOGGER.isDebugEnabled()) {
+                LOGGER.debug("Cache item not found: {}", fullUrl, e);

Review Comment:
   go with that. But as we are in the case of `ResourceDoesNotExistException` 
not sure why do we need to have a `if` and display the exception. 
   no if but just:
   ```
   } catch (ResourceDoesNotExistException e) {
     LOGGER.info("Cache item not found: {}", fullUrl);
   }
   ```
   
   We are just in the case of a resource not found so no need to print the 
exception the cause is really clear. I'm not sure to understand of printing a 
stack even in debug mode. Is there anything more interested in the stack?
   
   Frankly I don't even understand why an exception is used for that. There 
should be simply a boolean `not found` and not wasting CPU/memory by using an 
exception (but I know it's a different problem :))





> Do not print exception when probing builds in remote repo
> ---------------------------------------------------------
>
>                 Key: MBUILDCACHE-32
>                 URL: https://issues.apache.org/jira/browse/MBUILDCACHE-32
>             Project: Maven Build Cache Extension
>          Issue Type: Bug
>            Reporter: Alexander Ashitkin
>            Priority: Major
>              Labels: pull-request-available
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> When cache engine tries to discover existing cache by checksum, it sends get 
> request. 
> This request is normally getting 404s, because cache is not guaranteed to 
> exist.
> It's a normal situation and exception should not be printed in such case as 
> it meaninglessly pollutes logs:
> {code:java}
> org.apache.maven.wagon.ResourceDoesNotExistException: resource missing at 
> https://my-cache/.../buildinfo.xml, status: 404 Not Found
>     at 
> org.apache.maven.wagon.shared.http.AbstractHttpClientWagon.fillInputData 
> (AbstractHttpClientWagon.java:1191)
>     at 
> org.apache.maven.wagon.shared.http.AbstractHttpClientWagon.fillInputData 
> (AbstractHttpClientWagon.java:1140)
>     at org.apache.maven.wagon.StreamWagon.getInputStream 
> (StreamWagon.java:126)
>     at org.apache.maven.wagon.StreamWagon.getIfNewerToStream 
> (StreamWagon.java:226)
>     at org.apache.maven.wagon.StreamWagon.getToStream (StreamWagon.java:262)
>     at org.eclipse.aether.transport.wagon.WagonTransporter$GetTaskRunner.run 
> (WagonTransporter.java:533)
>     at org.eclipse.aether.transport.wagon.WagonTransporter.execute 
> (WagonTransporter.java:425)
>     at org.eclipse.aether.transport.wagon.WagonTransporter.get 
> (WagonTransporter.java:400)
>     at 
> org.apache.maven.buildcache.RemoteCacheRepositoryImpl.getResourceContent 
> (RemoteCacheRepositoryImpl.java:165)
>     at org.apache.maven.buildcache.RemoteCacheRepositoryImpl.findBuild 
> (RemoteCacheRepositoryImpl.java:114)
>     at org.apache.maven.buildcache.LocalCacheRepositoryImpl.findBuild 
> (LocalCacheRepositoryImpl.java:183)
>     at org.apache.maven.buildcache.CacheControllerImpl.findCachedBuild 
> (CacheControllerImpl.java:212)
>     at org.apache.maven.buildcache.CacheControllerImpl.findCachedBuild 
> (CacheControllerImpl.java:179)
>     at org.apache.maven.buildcache.BuildCacheMojosExecutionStrategy.execute 
> (BuildCacheMojosExecutionStrategy.java:114)
>     at org.apache.maven.lifecycle.internal.MojoExecutor.execute 
> (MojoExecutor.java:179)
>  {code}
> {{Need to create method similar to 
> RemoteCacheRepositoryImpl#getResourceContent, but }}{{getResourceContentQuiet 
> and use it when probing buildinfo.xml. the method should not log exceptions}}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to