gemini-code-assist[bot] commented on code in PR #38586:
URL: https://github.com/apache/beam/pull/38586#discussion_r3282185748
##########
settings.gradle.kts:
##########
@@ -18,6 +18,33 @@
import
com.gradle.enterprise.gradleplugin.internal.extension.BuildScanExtensionWithHiddenFeatures
pluginManagement {
+ val isMavenAccessible = try {
+ val connection =
java.net.URL("https://repo.maven.apache.org/maven2/org/slf4j/slf4j-api/1.7.2/slf4j-api-1.7.2.pom";).openConnection()
as java.net.HttpURLConnection
+ connection.requestMethod = "GET"
+ connection.setRequestProperty("User-Agent", "Gradle")
+ connection.connectTimeout = 2000
+ connection.readTimeout = 2000
+ connection.useCaches = false
+ connection.responseCode == 200
+ } catch (e: Exception) {
+ false
+ }
+ val mavenCentralMirrorUrl =
"https://maven-central.storage-download.googleapis.com/maven2/";
+
+ if (isMavenAccessible) {
+ logger.lifecycle("Maven Central is accessible. Using default
repositories for settings plugins.")
+ } else {
+ logger.lifecycle("Maven Central is not accessible. Falling back to
Google Maven Mirror for settings plugins.")
+ }
+
+ repositories {
+ if (isMavenAccessible) {
+ gradlePluginPortal()
+ } else {
+ maven { url = uri(mavenCentralMirrorUrl) }
+ gradlePluginPortal()
+ }
Review Comment:
The `gradlePluginPortal()` call is duplicated in both branches of the
accessibility check. Moving it outside the conditional block makes the logic
cleaner and easier to maintain.
```kotlin
if (!isMavenAccessible) {
maven { url = uri(mavenCentralMirrorUrl) }
}
gradlePluginPortal()
```
##########
buildSrc/build.gradle.kts:
##########
@@ -25,7 +52,12 @@ plugins {
// Define the set of repositories required to fetch and enable plugins.
repositories {
- maven { url = uri("https://plugins.gradle.org/m2/";) }
+ if (isMavenAccessible) {
+ maven { url = uri("https://plugins.gradle.org/m2/";) }
+ } else {
+ maven { url = uri(mavenCentralMirrorUrl) }
+ maven { url = uri("https://plugins.gradle.org/m2/";) }
+ }
Review Comment:
The declaration of the Gradle Plugin Portal repository is duplicated in both
branches of the conditional. This can be simplified by moving the common
repository outside the `if` block, improving maintainability.
```kotlin
if (!isMavenAccessible) {
maven { url = uri(mavenCentralMirrorUrl) }
}
maven { url = uri("https://plugins.gradle.org/m2/";) }
```
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/Repositories.groovy:
##########
@@ -68,8 +102,12 @@ class Repositories {
project.offlineDependencies {
repositories {
mavenLocal()
- mavenCentral()
- maven { url "https://plugins.gradle.org/m2/"; }
+ if (checkMavenCentral()) {
+ mavenCentral()
+ maven { url "https://plugins.gradle.org/m2/"; }
+ } else {
+ maven { url
"https://maven-central.storage-download.googleapis.com/maven2/"; }
+ }
Review Comment:
The current logic in the `offlineDependencies` block excludes the Gradle
Plugin Portal repository when Maven Central is inaccessible. Since the Google
Maven Mirror only mirrors Maven Central, any dependencies that are exclusively
hosted on the Gradle Plugin Portal will fail to resolve in this scenario. The
Plugin Portal should be included regardless of Maven Central's accessibility.
```
if (checkMavenCentral()) {
mavenCentral()
} else {
maven { url
"https://maven-central.storage-download.googleapis.com/maven2/"; }
}
maven { url "https://plugins.gradle.org/m2/"; }
```
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/Repositories.groovy:
##########
@@ -19,9 +19,39 @@
package org.apache.beam.gradle
import org.gradle.api.Project
+import java.net.HttpURLConnection
+import java.net.URL
class Repositories {
+ private static Boolean isMavenAccessible = null
+
+ static synchronized boolean checkMavenCentral() {
+ if (isMavenAccessible != null) {
+ return isMavenAccessible
+ }
+ def testUrl =
"https://repo.maven.apache.org/maven2/org/slf4j/slf4j-api/1.7.2/slf4j-api-1.7.2.pom";
+ try {
+ def url = new URL(testUrl)
+ def connection = (HttpURLConnection) url.openConnection()
+ connection.requestMethod = "GET"
+ connection.setRequestProperty("User-Agent", "Gradle")
+ connection.connectTimeout = 2000
+ connection.readTimeout = 2000
+ connection.useCaches = false
+ isMavenAccessible = (connection.responseCode == 200)
+ } catch (Exception e) {
+ isMavenAccessible = false
+ }
+
+ if (isMavenAccessible) {
+ println "Maven Central is accessible. Using default repositories."
+ } else {
+ println "Maven Central is not accessible. Falling back to Google Maven
Mirror."
+ }
+ return isMavenAccessible
+ }
Review Comment:
The logic for checking Maven Central accessibility and the associated URLs
are duplicated across `build.gradle.kts`, `Repositories.groovy`, and
`settings.gradle.kts`. While some duplication is expected due to the Gradle
lifecycle (e.g., `settings.gradle.kts` running before `buildSrc` is available),
the duplication within `buildSrc` itself could be reduced. Consider
centralizing the test URL and the mirror URL into a shared configuration or at
least using constants to avoid potential inconsistencies in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
