You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "talatuyarer (via GitHub)" <gi...@apache.org> on 2026/05/08 21:28:41 UTC
[PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
talatuyarer opened a new pull request, #16258:
URL: https://github.com/apache/iceberg/pull/16258
We dont want to load analytics classes if users does not use analytic feature.
https://lists.apache.org/thread/mv0xhgoqvj2doh74g4wr8pgft3ozj0hz
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#issuecomment-4434295487
Thanks for the fix @talatuyarer and everyone for reviewing
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3213856037
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/AnalyticsCoreUtil.java:
##########
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.gcp.gcs;
+
+import com.google.auth.Credentials;
+import com.google.cloud.gcs.analyticscore.client.GcsFileInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystem;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemImpl;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemOptions;
+import com.google.cloud.gcs.analyticscore.client.GcsItemId;
+import com.google.cloud.gcs.analyticscore.client.GcsItemInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsObjectRange;
+import com.google.cloud.gcs.analyticscore.core.GcsAnalyticsCoreOptions;
+import com.google.cloud.gcs.analyticscore.core.GoogleCloudStorageInputStream;
+import com.google.cloud.storage.BlobId;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.Map;
+import java.util.function.IntFunction;
+import java.util.stream.Collectors;
+import org.apache.iceberg.gcp.GCPProperties;
+import org.apache.iceberg.io.FileIOMetricsContext;
+import org.apache.iceberg.io.FileRange;
+import org.apache.iceberg.io.RangeReadable;
+import org.apache.iceberg.io.SeekableInputStream;
+import org.apache.iceberg.metrics.Counter;
+import org.apache.iceberg.metrics.MetricsContext;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.util.PropertyUtil;
+
+/**
+ * Gateway to the optional {@code com.google.cloud.gcs.analyticscore.*} dependency. All references
+ * to analytics-core types are confined to this class so that it is loaded only when {@link
+ * org.apache.iceberg.gcp.GCPProperties#GCS_ANALYTICS_CORE_ENABLED} is true.
+ */
+class AnalyticsCoreUtil {
+
+ private AnalyticsCoreUtil() {}
+
+ static AutoCloseable createFileSystem(Map<String, String> properties, Credentials credentials) {
Review Comment:
Here better to return GcsFileSystem rather than AutoCloseable.
##########
gcp/src/test/java/org/apache/iceberg/gcp/gcs/TestGcsInputFile.java:
##########
@@ -158,7 +158,7 @@ public void newStreamGcsAnalyticsCoreEnabled() throws IOException {
metricsContext);
try (SeekableInputStream stream = inputFile.newStream()) {
- assertThat(stream).isInstanceOf(GcsInputStreamWrapper.class);
+ assertThat(stream).isNotInstanceOf(GCSInputStream.class);
Review Comment:
nit: we can add the following to make sure GoogleCloudStorageInputStream is used
mocked.verify(() -> GoogleCloudStorageInputStream.create(gcsFileSystem, itemId));
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3212118936
##########
gcp/src/test/java/org/apache/iceberg/gcp/gcs/TestGcsInputStreamWrapper.java:
##########
Review Comment:
some of these tests might be useful to keep, maybe we can rename it to TestAnalyticsCoreUtil?
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,16 @@
abstract class BaseGCSFile {
private final Storage storage;
- private final GcsFileSystem gcsFileSystem;
+ // Using Object avoid a runtime dependency on gcs-analytics-core. Cast via AnalyticsCoreUtil.
+ private final Object gcsFileSystem;
Review Comment:
maybe AutoCloseable is better than Object?
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,16 @@
abstract class BaseGCSFile {
private final Storage storage;
- private final GcsFileSystem gcsFileSystem;
+ // Using Object avoid a runtime dependency on gcs-analytics-core. Cast via AnalyticsCoreUtil.
Review Comment:
```suggestion
// Using Object to avoid a runtime dependency on gcs-analytics-core. Cast via AnalyticsCoreUtil.
```
nit, this and all other places too
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "talatuyarer (via GitHub)" <gi...@apache.org>.
talatuyarer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211937764
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -115,23 +114,33 @@ public void close() {
}
}
+ if (null != gcsFileSystem) {
+ AnalyticsCoreUtil.close(gcsFileSystem);
+ gcsFileSystem = null;
+ }
+
if (null != storage) {
// GCS Storage does not appear to be closable, so release the reference
storage = null;
}
}
- GcsFileSystem gcsFileSystem() {
+ Object gcsFileSystem() {
Review Comment:
Done
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,15 @@
abstract class BaseGCSFile {
private final Storage storage;
- private final GcsFileSystem gcsFileSystem;
Review Comment:
Done
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/AnalyticsCoreUtil.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.gcp.gcs;
+
+import com.google.auth.Credentials;
+import com.google.cloud.gcs.analyticscore.client.GcsFileInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystem;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemImpl;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemOptions;
+import com.google.cloud.gcs.analyticscore.client.GcsItemId;
+import com.google.cloud.gcs.analyticscore.client.GcsItemInfo;
+import com.google.cloud.gcs.analyticscore.core.GcsAnalyticsCoreOptions;
+import com.google.cloud.gcs.analyticscore.core.GoogleCloudStorageInputStream;
+import com.google.cloud.storage.BlobId;
+import java.io.IOException;
+import java.net.URI;
+import java.util.Map;
+import org.apache.iceberg.io.SeekableInputStream;
+import org.apache.iceberg.metrics.MetricsContext;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+
+/**
+ * Gateway to the optional {@code com.google.cloud.gcs.analyticscore.*} dependency. All references
+ * to analytics-core types are confined to this class so that it is loaded only when {@link
+ * org.apache.iceberg.gcp.GCPProperties#GCS_ANALYTICS_CORE_ENABLED} is true.
+ */
+class AnalyticsCoreUtil {
+
+ private AnalyticsCoreUtil() {}
+
+ static Object createFileSystem(Map<String, String> properties, Credentials credentials) {
+ GcsAnalyticsCoreOptions options = new GcsAnalyticsCoreOptions("gcs.", properties);
Review Comment:
Done
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -59,6 +55,11 @@ class PrefixedStorage implements AutoCloseable {
this.storagePrefix = storagePrefix;
this.storage = storage;
this.gcpProperties = new GCPProperties(properties);
+ this.propertiesWithUserAgent =
+ ImmutableMap.<String, String>builder()
+ .putAll(properties)
+ .put("gcs.user-agent", GCS_FILE_IO_USER_AGENT)
+ .build();
Review Comment:
A collision can only happen if a caller pokes at undocumented analytics-core keys directly via the GCSFileIO property map. That's possible (the property map is open) but not on any documented or supported surface. Keep the current build() for this PR since making it buildKeepingLast() is a behavior change for an undocumented edge case, and I can add this change improvement PR.
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -45,11 +41,11 @@ class PrefixedStorage implements AutoCloseable {
private static final String GCS_FILE_IO_USER_AGENT = "gcsfileio/" + EnvironmentContext.get();
private final String storagePrefix;
private final GCPProperties gcpProperties;
+ private final Map<String, String> propertiesWithUserAgent;
private SerializableSupplier<Storage> storage;
private CloseableGroup closeableGroup;
private transient volatile Storage storageClient;
Review Comment:
I dont want to increase scope of work in this PR. I agree this is good improvement. I can create followup PR to address this
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "talatuyarer (via GitHub)" <gi...@apache.org>.
talatuyarer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3228600287
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/AnalyticsCoreUtil.java:
##########
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.gcp.gcs;
+
+import com.google.auth.Credentials;
+import com.google.cloud.gcs.analyticscore.client.GcsFileInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystem;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemImpl;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemOptions;
+import com.google.cloud.gcs.analyticscore.client.GcsItemId;
+import com.google.cloud.gcs.analyticscore.client.GcsItemInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsObjectRange;
+import com.google.cloud.gcs.analyticscore.core.GcsAnalyticsCoreOptions;
+import com.google.cloud.gcs.analyticscore.core.GoogleCloudStorageInputStream;
+import com.google.cloud.storage.BlobId;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.Map;
+import java.util.function.IntFunction;
+import java.util.stream.Collectors;
+import org.apache.iceberg.gcp.GCPProperties;
+import org.apache.iceberg.io.FileIOMetricsContext;
+import org.apache.iceberg.io.FileRange;
+import org.apache.iceberg.io.RangeReadable;
+import org.apache.iceberg.io.SeekableInputStream;
+import org.apache.iceberg.metrics.Counter;
+import org.apache.iceberg.metrics.MetricsContext;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.util.PropertyUtil;
+
+/**
+ * Gateway to the optional {@code com.google.cloud.gcs.analyticscore.*} dependency. All references
+ * to analytics-core types are confined to this class so that it is loaded only when {@link
+ * org.apache.iceberg.gcp.GCPProperties#GCS_ANALYTICS_CORE_ENABLED} is true.
+ */
+class AnalyticsCoreUtil {
+
+ private AnalyticsCoreUtil() {}
+
+ static AutoCloseable createFileSystem(Map<String, String> properties, Credentials credentials) {
Review Comment:
Returning `GcsFileSystem` would expose the analytics-core class in the method descriptor, which is the opposite of what we're trying to achieve (e.g., `PrefixedStorage.gcsFileSystem()`). Could you clarify if there's another reason for the suggestion? @aihuaxu
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211676320
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/AnalyticsCoreUtil.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.gcp.gcs;
+
+import com.google.auth.Credentials;
+import com.google.cloud.gcs.analyticscore.client.GcsFileInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystem;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemImpl;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemOptions;
+import com.google.cloud.gcs.analyticscore.client.GcsItemId;
+import com.google.cloud.gcs.analyticscore.client.GcsItemInfo;
+import com.google.cloud.gcs.analyticscore.core.GcsAnalyticsCoreOptions;
+import com.google.cloud.gcs.analyticscore.core.GoogleCloudStorageInputStream;
+import com.google.cloud.storage.BlobId;
+import java.io.IOException;
+import java.net.URI;
+import java.util.Map;
+import org.apache.iceberg.io.SeekableInputStream;
+import org.apache.iceberg.metrics.MetricsContext;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+
+/**
+ * Gateway to the optional {@code com.google.cloud.gcs.analyticscore.*} dependency. All references
+ * to analytics-core types are confined to this class so that it is loaded only when {@link
+ * org.apache.iceberg.gcp.GCPProperties#GCS_ANALYTICS_CORE_ENABLED} is true.
+ */
+class AnalyticsCoreUtil {
+
+ private AnalyticsCoreUtil() {}
+
+ static Object createFileSystem(Map<String, String> properties, Credentials credentials) {
+ GcsAnalyticsCoreOptions options = new GcsAnalyticsCoreOptions("gcs.", properties);
Review Comment:
You could add a precondition here rather than accepting it if the property is not set, this would guard against someone calling this function from a context where the properties hasn't been enabled.
```java
Preconditions.checkState(
PropertyUtil.propertyAsBoolean(
properties, GCPProperties.GCS_ANALYTICS_CORE_ENABLED, false),
"GCS analytics-core is disabled; %s must be set to true",
GCPProperties.GCS_ANALYTICS_CORE_ENABLED);
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211588919
##########
gcp/src/test/java/org/apache/iceberg/gcp/gcs/TestPrefixedStorage.java:
##########
@@ -115,17 +115,27 @@ public void impersonationPropertiesWithDefaults() {
.isEqualTo(GCPProperties.GCS_IMPERSONATE_LIFETIME_SECONDS_DEFAULT);
}
+ @Test
+ public void gcsFileSystemDisabledByDefault() {
+ Map<String, String> properties = ImmutableMap.of(GCPProperties.GCS_PROJECT_ID, "myProject");
+ PrefixedStorage storage = new PrefixedStorage("gs://bucket", properties, null);
+
+ assertThat(storage.gcsFileSystem()).isNull();
+ }
+
@Test
public void gcsFileSystem() {
Map<String, String> properties =
- ImmutableMap.of(
- GCPProperties.GCS_PROJECT_ID, "myProject",
- GCPProperties.GCS_USER_PROJECT, "userProject",
- GCPProperties.GCS_CLIENT_LIB_TOKEN, "gccl",
- GCPProperties.GCS_SERVICE_HOST, "example.com",
- GCPProperties.GCS_DECRYPTION_KEY, "decryptionKey",
- GCPProperties.GCS_ENCRYPTION_KEY, "encryptionKey",
- GCPProperties.GCS_CHANNEL_READ_CHUNK_SIZE, "1024");
+ ImmutableMap.<String, String>builder()
Review Comment:
I guess adding `GCPProperties.GCS_ANALYTICS_CORE_ENABLED, "true")` might have caused this to exceed the largest overload of `ImmutableMap.of`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211663441
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSOutputFile.java:
##########
@@ -44,7 +43,7 @@ static GCSOutputFile fromLocation(
GCSOutputFile(
Storage storage,
- GcsFileSystem gcsFileSystem,
+ Object gcsFileSystem,
Review Comment:
Best put an note here too
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "talatuyarer (via GitHub)" <gi...@apache.org>.
talatuyarer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3212460020
##########
gcp/src/test/java/org/apache/iceberg/gcp/gcs/TestGcsInputStreamWrapper.java:
##########
Review Comment:
I kept one test and created new Test File
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211676320
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/AnalyticsCoreUtil.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.gcp.gcs;
+
+import com.google.auth.Credentials;
+import com.google.cloud.gcs.analyticscore.client.GcsFileInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystem;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemImpl;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemOptions;
+import com.google.cloud.gcs.analyticscore.client.GcsItemId;
+import com.google.cloud.gcs.analyticscore.client.GcsItemInfo;
+import com.google.cloud.gcs.analyticscore.core.GcsAnalyticsCoreOptions;
+import com.google.cloud.gcs.analyticscore.core.GoogleCloudStorageInputStream;
+import com.google.cloud.storage.BlobId;
+import java.io.IOException;
+import java.net.URI;
+import java.util.Map;
+import org.apache.iceberg.io.SeekableInputStream;
+import org.apache.iceberg.metrics.MetricsContext;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+
+/**
+ * Gateway to the optional {@code com.google.cloud.gcs.analyticscore.*} dependency. All references
+ * to analytics-core types are confined to this class so that it is loaded only when {@link
+ * org.apache.iceberg.gcp.GCPProperties#GCS_ANALYTICS_CORE_ENABLED} is true.
+ */
+class AnalyticsCoreUtil {
+
+ private AnalyticsCoreUtil() {}
+
+ static Object createFileSystem(Map<String, String> properties, Credentials credentials) {
+ GcsAnalyticsCoreOptions options = new GcsAnalyticsCoreOptions("gcs.", properties);
Review Comment:
You could ad d a precondition here rather than accepting it if the property is not set, this would guard against someone calling this function from a context where the properties hasn't been enabled.
```java
Preconditions.checkState(
properties.isGcsAnalyticsCoreEnabled(),
"GCS analytics-core feature is disabled; cannot create GcsFileSystem");
```
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/AnalyticsCoreUtil.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.gcp.gcs;
+
+import com.google.auth.Credentials;
+import com.google.cloud.gcs.analyticscore.client.GcsFileInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystem;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemImpl;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemOptions;
+import com.google.cloud.gcs.analyticscore.client.GcsItemId;
+import com.google.cloud.gcs.analyticscore.client.GcsItemInfo;
+import com.google.cloud.gcs.analyticscore.core.GcsAnalyticsCoreOptions;
+import com.google.cloud.gcs.analyticscore.core.GoogleCloudStorageInputStream;
+import com.google.cloud.storage.BlobId;
+import java.io.IOException;
+import java.net.URI;
+import java.util.Map;
+import org.apache.iceberg.io.SeekableInputStream;
+import org.apache.iceberg.metrics.MetricsContext;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+
+/**
+ * Gateway to the optional {@code com.google.cloud.gcs.analyticscore.*} dependency. All references
+ * to analytics-core types are confined to this class so that it is loaded only when {@link
+ * org.apache.iceberg.gcp.GCPProperties#GCS_ANALYTICS_CORE_ENABLED} is true.
+ */
+class AnalyticsCoreUtil {
+
+ private AnalyticsCoreUtil() {}
+
+ static Object createFileSystem(Map<String, String> properties, Credentials credentials) {
+ GcsAnalyticsCoreOptions options = new GcsAnalyticsCoreOptions("gcs.", properties);
Review Comment:
You could add a precondition here rather than accepting it if the property is not set, this would guard against someone calling this function from a context where the properties hasn't been enabled.
```java
Preconditions.checkState(
properties.isGcsAnalyticsCoreEnabled(),
"GCS analytics-core feature is disabled; cannot create GcsFileSystem");
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211823660
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,15 @@
abstract class BaseGCSFile {
private final Storage storage;
- private final GcsFileSystem gcsFileSystem;
Review Comment:
is it better to use `AutoCloseable` instead of `Object`? (this is a claude suggestion)
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -59,6 +55,11 @@ class PrefixedStorage implements AutoCloseable {
this.storagePrefix = storagePrefix;
this.storage = storage;
this.gcpProperties = new GCPProperties(properties);
+ this.propertiesWithUserAgent =
+ ImmutableMap.<String, String>builder()
+ .putAll(properties)
+ .put("gcs.user-agent", GCS_FILE_IO_USER_AGENT)
+ .build();
Review Comment:
nit: If the caller's properties already contains "gcs.user-agent", ImmutableMap.Builder.put() will throw IllegalArgumentException for duplicate keys. This existed in the old code too, but now the map is built eagerly (always) rather than lazily (only when creating the supplier). Consider using buildKeepingLast() or filtering the key before putAll.
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -115,23 +114,33 @@ public void close() {
}
}
+ if (null != gcsFileSystem) {
+ AnalyticsCoreUtil.close(gcsFileSystem);
+ gcsFileSystem = null;
+ }
+
Review Comment:
If AnalyticsCoreUtil.close() throws (e.g., the underlying GcsFileSystem.close() fails), the storage = null cleanup is skipped. The old code added gcsFileSystem to closeableGroup, which handles multi-resource cleanup safely. Consider wrapping in try-finally
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211659074
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -115,23 +114,33 @@ public void close() {
}
}
+ if (null != gcsFileSystem) {
+ AnalyticsCoreUtil.close(gcsFileSystem);
+ gcsFileSystem = null;
+ }
+
if (null != storage) {
// GCS Storage does not appear to be closable, so release the reference
storage = null;
}
}
- GcsFileSystem gcsFileSystem() {
+ Object gcsFileSystem() {
Review Comment:
Similar to @emkornfield 's comment above add a doc here explaining why we are guarding the return type
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211497837
##########
gcp/src/test/java/org/apache/iceberg/gcp/gcs/TestPrefixedStorage.java:
##########
@@ -115,17 +115,27 @@ public void impersonationPropertiesWithDefaults() {
.isEqualTo(GCPProperties.GCS_IMPERSONATE_LIFETIME_SECONDS_DEFAULT);
}
+ @Test
+ public void gcsFileSystemDisabledByDefault() {
+ Map<String, String> properties = ImmutableMap.of(GCPProperties.GCS_PROJECT_ID, "myProject");
+ PrefixedStorage storage = new PrefixedStorage("gs://bucket", properties, null);
+
+ assertThat(storage.gcsFileSystem()).isNull();
+ }
+
@Test
public void gcsFileSystem() {
Map<String, String> properties =
- ImmutableMap.of(
- GCPProperties.GCS_PROJECT_ID, "myProject",
- GCPProperties.GCS_USER_PROJECT, "userProject",
- GCPProperties.GCS_CLIENT_LIB_TOKEN, "gccl",
- GCPProperties.GCS_SERVICE_HOST, "example.com",
- GCPProperties.GCS_DECRYPTION_KEY, "decryptionKey",
- GCPProperties.GCS_ENCRYPTION_KEY, "encryptionKey",
- GCPProperties.GCS_CHANNEL_READ_CHUNK_SIZE, "1024");
+ ImmutableMap.<String, String>builder()
Review Comment:
nit: unnecessary change
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211662177
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSInputFile.java:
##########
@@ -57,7 +50,7 @@ static GCSInputFile fromLocation(
GCSInputFile(
Storage storage,
- GcsFileSystem gcsFileSystem,
+ Object gcsFileSystem,
Review Comment:
Like @emkornfield said, probably worth an annotation here or comment
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "talatuyarer (via GitHub)" <gi...@apache.org>.
talatuyarer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3212459471
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,16 @@
abstract class BaseGCSFile {
private final Storage storage;
- private final GcsFileSystem gcsFileSystem;
+ // Using Object avoid a runtime dependency on gcs-analytics-core. Cast via AnalyticsCoreUtil.
Review Comment:
Done
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #16258:
URL: https://github.com/apache/iceberg/pull/16258
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211791747
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -45,11 +41,11 @@ class PrefixedStorage implements AutoCloseable {
private static final String GCS_FILE_IO_USER_AGENT = "gcsfileio/" + EnvironmentContext.get();
private final String storagePrefix;
private final GCPProperties gcpProperties;
+ private final Map<String, String> propertiesWithUserAgent;
private SerializableSupplier<Storage> storage;
private CloseableGroup closeableGroup;
private transient volatile Storage storageClient;
Review Comment:
nit: The `storageClient` field is only used in gcsFileSystem() when analytics-core is enabled, but it's allocated for every PrefixedStorage instance. The old code built this map only inside the supplier. Consider building it lazily inside gcsFileSystem() or only when gcpProperties.isGcsAnalyticsCoreEnabled() is true
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3211586432
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,15 @@
abstract class BaseGCSFile {
private final Storage storage;
- private final GcsFileSystem gcsFileSystem;
Review Comment:
It might be worth a note someplace on why these are Objects instead of the desired type.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#issuecomment-4410358078
> i grepped for `import.*analyticscore`, looks like theres still an import in `gcp/src/main/java/org/apache/iceberg/gcp/gcs/GcsInputStreamWrapper.java` i would expect all the imports for analytics-core isolated in AnalyticsCoreUtil.java
>
> EDIT: ok looks like `GcsInputStreamWrapper` is only called by `AnalyticsCoreUtil`. no issues here
This is actually a good call, you might as well make GcsinputStreamwrapper an inner class of AnalyticsCoreUtil and remove the possibility of another caller accidentally using it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "talatuyarer (via GitHub)" <gi...@apache.org>.
talatuyarer commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3212456845
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -115,23 +114,33 @@ public void close() {
}
}
+ if (null != gcsFileSystem) {
+ AnalyticsCoreUtil.close(gcsFileSystem);
+ gcsFileSystem = null;
+ }
+
Review Comment:
Done
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Move all analyticscore references behind a AnalyticsCoreUtil class [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3228676274
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/AnalyticsCoreUtil.java:
##########
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.gcp.gcs;
+
+import com.google.auth.Credentials;
+import com.google.cloud.gcs.analyticscore.client.GcsFileInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystem;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemImpl;
+import com.google.cloud.gcs.analyticscore.client.GcsFileSystemOptions;
+import com.google.cloud.gcs.analyticscore.client.GcsItemId;
+import com.google.cloud.gcs.analyticscore.client.GcsItemInfo;
+import com.google.cloud.gcs.analyticscore.client.GcsObjectRange;
+import com.google.cloud.gcs.analyticscore.core.GcsAnalyticsCoreOptions;
+import com.google.cloud.gcs.analyticscore.core.GoogleCloudStorageInputStream;
+import com.google.cloud.storage.BlobId;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.Map;
+import java.util.function.IntFunction;
+import java.util.stream.Collectors;
+import org.apache.iceberg.gcp.GCPProperties;
+import org.apache.iceberg.io.FileIOMetricsContext;
+import org.apache.iceberg.io.FileRange;
+import org.apache.iceberg.io.RangeReadable;
+import org.apache.iceberg.io.SeekableInputStream;
+import org.apache.iceberg.metrics.Counter;
+import org.apache.iceberg.metrics.MetricsContext;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.util.PropertyUtil;
+
+/**
+ * Gateway to the optional {@code com.google.cloud.gcs.analyticscore.*} dependency. All references
+ * to analytics-core types are confined to this class so that it is loaded only when {@link
+ * org.apache.iceberg.gcp.GCPProperties#GCS_ANALYTICS_CORE_ENABLED} is true.
+ */
+class AnalyticsCoreUtil {
+
+ private AnalyticsCoreUtil() {}
+
+ static AutoCloseable createFileSystem(Map<String, String> properties, Credentials credentials) {
Review Comment:
@talatuyarer I think you can return `GcsFileSystem` here but the caller can still use `AutoCloseable`, right?
But it's nit.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org