github-actions[bot] commented on code in PR #68540:
URL: https://github.com/apache/doris/pull/68540#discussion_r4246097904
##########
fe/be-java-extensions/iceberg-metadata-scanner/pom.xml:
##########
@@ -41,6 +41,18 @@ under the License.
</properties>
<dependencies>
+ <!-- JNI plugins cannot inherit the GCS connector from BE's
hadoop_hdfs classpath. -->
+ <dependency>
+ <groupId>com.google.cloud.bigdataoss</groupId>
+ <artifactId>gcs-connector</artifactId>
+ <classifier>shaded</classifier>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.doris</groupId>
+ <artifactId>fe-iceberg-common</artifactId>
+ <version>${revision}</version>
Review Comment:
[P1] Keep FE-only wire codecs out of the BE scanner closure
This dependency transitively ships `fe-filesystem-gcs`, including
`GcsCredentialCodec`, but its `fe-grpc` and `fe-thrift` dependencies are
`provided` and neither is supplied by the scanner runtime graph.
`check_plugin_layout.py` analyzes every Doris-owned JAR, so the codec's
references to `Cloud$GcpCredentialPB`, `Cloud$ObjectStoreCredentialPB` and
`TCredential`/`TGcpCredential` are unresolved even though the scanner does not
call the codec. `build.sh` treats these `closure-self-contained` errors as
fatal, making a standard BE build with Java extensions fail during packaging.
Split the reusable authentication/client code from the FE codec, provide the
missing closure, or add a precise justified exception for unused codec
references. This is distinct from the earlier missing-factory thread: that
runtime factory is now present; the newly bundled FE codec breaks the build
gate.
##########
fe/fe-filesystem/fe-filesystem-gcs/src/main/java/org/apache/doris/filesystem/gcs/GcsFileSystemProvider.java:
##########
@@ -36,14 +41,113 @@
*
* <p>Selected explicitly by {@code provider=GCS}/{@code GCP} or {@code
fs.gcs.support=true};
* otherwise, when the user declared no filesystem explicitly, by
- * {@link S3CompatSignals#guessIsGcs} (a {@code gs.endpoint}, or an endpoint on
+ * {@link GcsAuthResolver#guessIsGcs} (a {@code gs.endpoint}, or an endpoint on
* {@code storage.googleapis.com}). Delegates I/O to {@link S3FileSystem}.
*/
public class GcsFileSystemProvider implements
FileSystemProvider<GcsFileSystemProperties> {
private static final String STORAGE_TYPE_GCS = "GCS";
private static final String FS_GCS_SUPPORT = "fs.gcs.support";
+ private static final Map<String, String> RESOURCE_ALIASES = Map.ofEntries(
+ Map.entry("gs.endpoint", "s3.endpoint"),
+ Map.entry("gs.access_key", "s3.access_key"),
+ Map.entry("gs.secret_key", "s3.secret_key"),
+ Map.entry("gs.session_token", "s3.session_token"),
+ Map.entry("gs.connection.maximum", "s3.connection.maximum"),
+ Map.entry("gs.connection.request.timeout",
"s3.connection.request.timeout"),
+ Map.entry("gs.connection.timeout", "s3.connection.timeout"),
+ Map.entry("gs.use_path_style", "use_path_style"),
+ Map.entry("gs.force_parsing_by_standard_uri",
"force_parsing_by_standard_uri"));
+
+ @Override
+ public Map<String, String> normalizeProperties(Map<String, String>
properties, Map<String, String> context) {
+ GcsAuthResolver.resolve(properties);
+ Map<String, String> normalized = new HashMap<>(properties);
+ String selected = context.get("provider");
+ if (selected == null || selected.isBlank()) {
+ if (!GcsAuthResolver.guessIsGcs(context)) {
+ normalized.keySet().removeAll(RESOURCE_ALIASES.keySet());
+ return normalized;
+ }
+ selected = "GCP";
+ normalized.put("provider", selected);
+ }
+ if ("GCP".equalsIgnoreCase(selected)) {
+ RESOURCE_ALIASES.forEach((alias, key) -> {
+ String value = normalized.remove(alias);
+ if (value != null && !value.isBlank()) {
Review Comment:
[P2] Preserve explicit session-token clears during alias normalization
For a `provider=GCP` resource storing HMAC keys and a nonempty
`gs.session_token` (with `s3_validity_check=false`), `ALTER RESOURCE ...
PROPERTIES("gs.session_token"="")` removes the alias here without adding its
canonical key. `S3Resource.modifyProperties` therefore never reaches its
session-token clear branch: the ALTER succeeds and bumps the version while
`s3.session_token` and the token pushed to BEs remain unchanged. The equivalent
`s3.session_token=""` clears correctly. Preserve the explicit empty value for
this clearable alias and cover the CREATE/ALTER/readback and Thrift path.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
