try 1.11.0 rc1#42
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Iceberg version to 1.11 and introduces a configurable Maven mirror for Iceberg JARs. The reviewer recommends using the full semantic version 1.11.0 for proper artifact resolution and suggests defaulting the mirror URL to the standard Maven repository instead of hardcoding a staging URL to improve maintainability.
| # Dependency versions - keep these compatible | ||
| # Changing these will invalidate the JAR download cache layer | ||
| ARG ICEBERG_VERSION=1.10.1 | ||
| ARG ICEBERG_VERSION=1.11 |
There was a problem hiding this comment.
The version 1.11 appears to be incomplete given the PR title "try 1.11.0 rc1". Apache Iceberg typically follows semantic versioning (x.y.z). If this is for the 1.11.0 release candidate, the version should likely be 1.11.0 to correctly resolve the artifact paths in the Maven repository.
ARG ICEBERG_VERSION=1.11.0
| # ARG ICEBERG_MAVEN_MIRROR=${MAVEN_MIRROR} | ||
| ARG ICEBERG_MAVEN_MIRROR=https://repository.apache.org/content/repositories/orgapacheiceberg-1278 |
There was a problem hiding this comment.
Hardcoding a specific staging repository URL makes the Dockerfile less flexible and harder to maintain once the release is finalized. It is better to define ICEBERG_MAVEN_MIRROR with a default value pointing to the standard mirror and override it via a build argument (--build-arg) when testing release candidates.
ARG ICEBERG_MAVEN_MIRROR=${MAVEN_MIRROR}
Co-authored-by: Copilot <copilot@github.com>
f8cc523 to
b819d4b
Compare
Rationale for this change
Are these changes tested?
Are there any user-facing changes?