You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2023/01/30 15:08:00 UTC

[jira] [Commented] (MBUILDCACHE-32) Do not print exception when probing builds in remote repo

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

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

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


##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -165,9 +168,28 @@ public Optional<byte[]> getResourceContent( String url ) throws IOException
             transporter.get( task );
             return Optional.of( task.getDataBytes() );
         }
+        catch ( ResourceDoesNotExistException e )
+        {
+            if ( mavenSession.getRequest().isShowErrors() )
+            {
+                LOGGER.info( "Resource not found: {}", getFullUrl( url ), e );
+            }
+            else
+            {
+                LOGGER.info( "Resource not found: {}", getFullUrl( url ) );
+            }
+            return Optional.empty();
+        }
         catch ( Exception e )
         {
-            LOGGER.info( "Cannot download {}", getFullUrl( url ), e );
+            if ( mavenSession.getRequest().isShowErrors() )
+            {
+                LOGGER.error( "Cannot download {}", getFullUrl( url ), e );

Review Comment:
   Afaik, printing an error usually means the build should fail, or the user should really look at the problem.  If the cache isn't available, I don't think this warrant an error, as the build should continue locally instead of downloading the already built artifacts.  I would tone it down to `warn` at least.



##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -165,9 +168,28 @@ public Optional<byte[]> getResourceContent( String url ) throws IOException
             transporter.get( task );
             return Optional.of( task.getDataBytes() );
         }
+        catch ( ResourceDoesNotExistException e )
+        {
+            if ( mavenSession.getRequest().isShowErrors() )

Review Comment:
   I think it would be better to use `LOGGER.isDebugEnabled()` rather than the request parameter.



##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -64,6 +65,7 @@ public class RemoteCacheRepositoryImpl implements RemoteCacheRepository, Closeab
 
     private final XmlService xmlService;
     private final CacheConfig cacheConfig;
+    private final MavenSession mavenSession;

Review Comment:
   This should not be needed anymore (see below).



##########
src/site/markdown/getting-started.md:
##########
@@ -17,7 +17,7 @@
 
 ## Getting Started
 
-To on-board incremental Maven you need to complete several steps:
+To on-board incremental Maven need to complete several steps:

Review Comment:
   From a dev perspective, having atomic commits is better imho.  The doc should be split from the code change are those are completely unrelated.



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.

Review Comment:
   There are very few differences imho.  Do you foresee any change on that part ?



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.
 
 ### Adding build cache config
 
 Copy template config [`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
 to [`.mvn/`](https://maven.apache.org/configure.html) directory of your project.  
-To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In a

Review Comment:
   Missing articles. `To get a comprehensive understanding of the build cache machinery`



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.
 
 ### Adding build cache config
 
 Copy template config [`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
 to [`.mvn/`](https://maven.apache.org/configure.html) directory of your project.  
-To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In a
+typical scenario need to:
 
 * Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to reconciliation
-* Configure precise source code files selectors. Though source code locations discovered automatically from project and plugins config,
-  there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations discovered automatically from project and

Review Comment:
   Missing verb.  `Although source code locations are discovered automatically from the project configuration and plugins, there may be edge cases.`



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.
 
 ### Adding build cache config
 
 Copy template config [`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
 to [`.mvn/`](https://maven.apache.org/configure.html) directory of your project.  
-To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In a
+typical scenario need to:
 
 * Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to reconciliation
-* Configure precise source code files selectors. Though source code locations discovered automatically from project and plugins config,
-  there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations discovered automatically from project and
+  plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)
 
 ### Adjusting build cache config
 
-Having extension run usual command, like `mvn package`. Verify the caching engine is activated:
+Having extension configured run usual command, like `mvn package`. Verify the caching engine is activated:

Review Comment:
   Missing verb.  Not even sure what is meant here with `Having extension configured run usual command` ...



##########
src/site/markdown/remote-cache.md:
##########
@@ -29,12 +29,11 @@ Before you start, please keep in mind basic principles:
   source code file, every dependency and effective pom (including plugin parameters). Every element's hash contributes
   to the key. In order to produce the same key there engine must consume exactly the same hashes.
 * There is no built-in normalization of line endings in this implementation, file hash calculation is raw bytes
-  based. The most obvious implication could be illustrated by a simple Git checkout. By default, git will check out
-  source code with CRLF line endings on win and LF on Linux. Because of that builds over same commit on a Linux agent
-  and local build on Windows workstation will yield different hashes.
+  based. Common pitfall is a Git checkout with CRLF on Win and LF on Linux. Same commit with different line endings will

Review Comment:
   `The same commit`



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.
 
 ### Adding build cache config
 
 Copy template config [`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
 to [`.mvn/`](https://maven.apache.org/configure.html) directory of your project.  
-To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In a
+typical scenario need to:
 
 * Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to reconciliation
-* Configure precise source code files selectors. Though source code locations discovered automatically from project and plugins config,
-  there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations discovered automatically from project and
+  plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)
 
 ### Adjusting build cache config
 
-Having extension run usual command, like `mvn package`. Verify the caching engine is activated:
+Having extension configured run usual command, like `mvn package`. Verify the caching engine is activated:
 
-* Check log output - there should be cache related output or initialization error message:
+* Check log output - there should be the cache related output or initialization error message:
   ```
   [INFO] Loading cache configuration from <project dir>/.mvn/maven-build-cache-config.xml
   ```
 * Navigate to your local repo directory - there should be a sibling directory `cache` next to the usual
   local `repository`.
 * Find `buildinfo.xml` in the cache repository for typical module and review it. Ensure that
-  * expected source code files are present in the build info
-  * Review all plugings used in the build and add their critical parameters to reconciliation
+    * Expected source code files are present in the build info

Review Comment:
   lowercase is preferred I think, as it's the same sentence. Missing article ? `the expected source code`



##########
src/site/markdown/remote-cache.md:
##########
@@ -88,13 +99,12 @@ Allow writes in remote cache add jvm property to designated CI builds.
 Run the build, review log and ensure that artifacts are uploaded to remote cache. Now, rerun build and ensure that it
 completes almost instantly because it is fully cached.
 
-### Remote cache relocation to local builds
+### Make remote cache portable
 
-As practice shows, developers often don't realize that builds they run in local and CI environments are different. So
-straightforward attempt to reuse remote cache in local build usually results in cache misses because of difference in
-plugins, parameters, profiles, environment, etc. In order to reuse results you might need to change poms, cache config,
-CI jobs and the project itself. This part is usually most challenging and time-consuming. Follow steps below to
-iteratively achieve working configuration.
+As practice shows, builds which run on local workstations and CI environments are different. Straightforward attempt to
+reuse cache produced in different environment usually results in cache misses. As is, caches are rarely portable because
+of differences in CI jobs, environment specifics and even project itself. Making cache portable is usually the most

Review Comment:
   `even the project`
   `Making caches portable`



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.
 
 ### Adding build cache config
 
 Copy template config [`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
 to [`.mvn/`](https://maven.apache.org/configure.html) directory of your project.  
-To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In a
+typical scenario need to:

Review Comment:
   Missing pronoun.



##########
src/site/markdown/remote-cache.md:
##########
@@ -112,7 +122,7 @@ Copy the link to a `build-cache-report.xml` and provide it to your local build a
 mvn verify -Dmaven.build.cache.failFast=true -Dmaven.build.cache.baselineUrl=https://your-cache-url/<...>/915296a3-4596-4eb5-bf37-f6e13ebe087e/build-cache-report.xml
 ```
 
-Once discrepancy between remote and local builds detected cache will fail with diagnostic info in
+Once discrepancy between remote and local builds detected cache will fail and spool diagnostic info in

Review Comment:
   `Once the discrepancy between remote and local versions is detected, the cache will fail and diagnostic information will be stored in`



##########
src/main/java/org/apache/maven/buildcache/RemoteCacheRepositoryImpl.java:
##########
@@ -165,9 +168,28 @@ public Optional<byte[]> getResourceContent( String url ) throws IOException
             transporter.get( task );
             return Optional.of( task.getDataBytes() );
         }
+        catch ( ResourceDoesNotExistException e )
+        {
+            if ( mavenSession.getRequest().isShowErrors() )
+            {
+                LOGGER.info( "Resource not found: {}", getFullUrl( url ), e );
+            }
+            else
+            {
+                LOGGER.info( "Resource not found: {}", getFullUrl( url ) );
+            }
+            return Optional.empty();
+        }
         catch ( Exception e )
         {
-            LOGGER.info( "Cannot download {}", getFullUrl( url ), e );
+            if ( mavenSession.getRequest().isShowErrors() )
+            {
+                LOGGER.error( "Cannot download {}", getFullUrl( url ), e );

Review Comment:
   Use `LOGGER.isDebugEnabled()` rather than `mavenSession.getRequest().isShowErrors()`.



##########
src/site/markdown/getting-started.md:
##########
@@ -17,7 +17,7 @@
 
 ## Getting Started
 
-To on-board incremental Maven you need to complete several steps:
+To on-board incremental Maven need to complete several steps:

Review Comment:
   There's no pronoun anymore...



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.
 
 ### Adding build cache config
 
 Copy template config [`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
 to [`.mvn/`](https://maven.apache.org/configure.html) directory of your project.  
-To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In a
+typical scenario need to:
 
 * Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to reconciliation
-* Configure precise source code files selectors. Though source code locations discovered automatically from project and plugins config,
-  there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations discovered automatically from project and
+  plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)

Review Comment:
   May be simplify to `Configure remote cache if needed`



##########
src/site/markdown/getting-started.md:
##########
@@ -27,48 +27,55 @@ To on-board incremental Maven you need to complete several steps:
 ### Declaring build cache extension
 
 ```xml
+
 <extension>
     <groupId>org.apache.maven.extensions</groupId>
     <artifactId>maven-build-cache-extension</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.0</version>
 </extension>
 ```
 
-either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`
+either in `pom.xml`'s `<project>/<build>/<extensions>` or in `.mvn/extensions.xml`'s `<extensions>`. Using core
+extension model (`.mvn/extensions.xml` file) is preferable as it allows better access to maven apis and could allow
+more sophisticated optimizations in the future.
 
 ### Adding build cache config
 
 Copy template config [`maven-build-cache-config.xml`](../resources/maven-build-cache-config.xml)
 to [`.mvn/`](https://maven.apache.org/configure.html) directory of your project.  
-To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In typical
-scenario you need to:
+To get overall understanding of build cache machinery, it is recommended to review the config and read comments. In a
+typical scenario need to:
 
 * Exclude unstable, temporary files or environment specific files
-* Add plugins reconciliation rules – add critical plugins parameters to reconciliation
-* Configure precise source code files selectors. Though source code locations discovered automatically from project and plugins config,
-  there might be edge cases.
-* Add remote cache location (if remote cache is used)
+* Add critical plugins parameters to runtime reconciliation
+* Configure precise source code files selectors. Though source code locations discovered automatically from project and
+  plugins config, there might be edge cases.
+* Configure remote cache (if remote cache is used)
 
 ### Adjusting build cache config
 
-Having extension run usual command, like `mvn package`. Verify the caching engine is activated:
+Having extension configured run usual command, like `mvn package`. Verify the caching engine is activated:
 
-* Check log output - there should be cache related output or initialization error message:
+* Check log output - there should be the cache related output or initialization error message:

Review Comment:
   `a` rather than `the` ? 
   same for `or an initialization error message` ?



##########
src/site/markdown/remote-cache.md:
##########
@@ -57,19 +56,31 @@ supports http PUT/GET/HEAD operations will suffice (Nginx, Apache or similar). A
 change `remote@enabled` to true:
 
 ```xml
-<remote enabled="true">
+<!--Default @id is 'cache'-->
+<remote enabled="true" id="my-cache">
     <url>http://your-buildcache-url</url>
 </remote>
 ```
 
 If proxy or authentication is required to access remote cache, add server record to settings.xml as described
 in [Servers](https://maven.apache.org/settings.html#Servers). The server should be referenced from cache config:
 
-```
-TBD
+```xml
+
+<servers>
+  <server>
+    <!

> 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)