gemini-code-assist[bot] commented on code in PR #38586:
URL: https://github.com/apache/beam/pull/38586#discussion_r3282670233
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/Repositories.groovy:
##########
@@ -19,10 +19,51 @@
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(Project project) {
+ if (isMavenAccessible != null) {
+ return isMavenAccessible
+ }
Review Comment:
To avoid redundant network checks, consider reusing the result of the
connectivity test performed during the `buildSrc` initialization phase, which
is stored in the `buildSrc.maven.accessible` system property.
```
if (isMavenAccessible != null) {
return isMavenAccessible
}
def cached = System.getProperty("buildSrc.maven.accessible")
if (cached != null) {
isMavenAccessible = Boolean.valueOf(cached)
return isMavenAccessible
}
```
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/Repositories.groovy:
##########
@@ -19,10 +19,51 @@
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(Project project) {
+ if (isMavenAccessible != null) {
+ return isMavenAccessible
+ }
+ def testUrl = project.findProperty("mavenCentralTestUrl")
+ if (!testUrl) {
+ isMavenAccessible = true
+ return isMavenAccessible
+ }
+ 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
+ }
+
+ def mirrorUrl = project.findProperty("mavenCentralMirrorUrl")
+ def useFallback = !isMavenAccessible && mirrorUrl
+
+ if (isMavenAccessible) {
+ println "Maven Central is accessible. Using default repositories."
+ } else if (useFallback) {
+ println "Maven Central is not accessible. Redirecting to Google Maven
Mirror."
+ } else {
+ println "Maven Central is not accessible and no mirror is configured.
Using default repositories."
+ }
Review Comment:
Use `project.logger.lifecycle` instead of `println` for better integration
with Gradle's logging system.
```
if (isMavenAccessible) {
project.logger.lifecycle "Maven Central is accessible. Using default
repositories."
} else if (useFallback) {
project.logger.lifecycle "Maven Central is not accessible. Redirecting
to Google Maven Mirror."
} else {
project.logger.lifecycle "Maven Central is not accessible and no
mirror is configured. Using default repositories."
}
```
##########
settings.gradle.kts:
##########
@@ -18,6 +18,41 @@
import
com.gradle.enterprise.gradleplugin.internal.extension.BuildScanExtensionWithHiddenFeatures
pluginManagement {
+ val mavenCentralTestUrl =
settings.providers.gradleProperty("mavenCentralTestUrl").orNull
+ val mavenCentralMirrorUrl =
settings.providers.gradleProperty("mavenCentralMirrorUrl").orNull
+
+ val isMavenAccessible = if (!mavenCentralTestUrl.isNullOrBlank()) {
Review Comment:
To avoid redundant network checks, consider reusing the result of the
connectivity test performed during the `buildSrc` initialization phase.
```kotlin
val isMavenAccessible =
System.getProperty("buildSrc.maven.accessible")?.toBoolean() ?: if
(!mavenCentralTestUrl.isNullOrBlank()) {
```
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/Repositories.groovy:
##########
@@ -19,10 +19,51 @@
package org.apache.beam.gradle
import org.gradle.api.Project
+import java.net.HttpURLConnection
+import java.net.URL
Review Comment:
Add `java.net.URI` to the imports to support a more robust way of creating
URLs, avoiding the deprecated `URL(String)` constructor.
```
import java.net.HttpURLConnection
import java.net.URL
import java.net.URI
```
##########
settings.gradle.kts:
##########
@@ -18,6 +18,41 @@
import
com.gradle.enterprise.gradleplugin.internal.extension.BuildScanExtensionWithHiddenFeatures
pluginManagement {
+ val mavenCentralTestUrl =
settings.providers.gradleProperty("mavenCentralTestUrl").orNull
+ val mavenCentralMirrorUrl =
settings.providers.gradleProperty("mavenCentralMirrorUrl").orNull
+
+ val isMavenAccessible = if (!mavenCentralTestUrl.isNullOrBlank()) {
+ try {
+ val connection =
java.net.URL(mavenCentralTestUrl).openConnection() as java.net.HttpURLConnection
Review Comment:
The `URL(String)` constructor is deprecated since Java 20. Use
`java.net.URI(mavenCentralTestUrl).toURL()` instead.
```kotlin
val connection =
java.net.URI(mavenCentralTestUrl).toURL().openConnection() as
java.net.HttpURLConnection
```
##########
buildSrc/settings.gradle.kts:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.util.Properties
+
+pluginManagement {
+ val parentProperties = java.util.Properties().apply {
+ val file = file("../gradle.properties")
+ if (file.exists()) {
+ file.inputStream().use { load(it) }
+ }
+ }
+
+ val mavenCentralTestUrl =
parentProperties.getProperty("mavenCentralTestUrl")
+ val mavenCentralMirrorUrl =
parentProperties.getProperty("mavenCentralMirrorUrl")
+
+ val isMavenAccessible = if (!mavenCentralTestUrl.isNullOrBlank()) {
+ try {
+ val connection =
java.net.URL(mavenCentralTestUrl).openConnection() as java.net.HttpURLConnection
Review Comment:
The `URL(String)` constructor is deprecated since Java 20. Use
`java.net.URI(mavenCentralTestUrl).toURL()` instead.
```kotlin
val connection =
java.net.URI(mavenCentralTestUrl).toURL().openConnection() as
java.net.HttpURLConnection
```
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/Repositories.groovy:
##########
@@ -19,10 +19,51 @@
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(Project project) {
+ if (isMavenAccessible != null) {
+ return isMavenAccessible
+ }
+ def testUrl = project.findProperty("mavenCentralTestUrl")
+ if (!testUrl) {
+ isMavenAccessible = true
+ return isMavenAccessible
+ }
+ try {
+ def url = new URL(testUrl)
Review Comment:
The `URL(String)` constructor is deprecated since Java 20. Use `new
URI(testUrl).toURL()` instead.
```
def url = new URI(testUrl).toURL()
```
--
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]
