Remove legacy object storage file systems#28936
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Removes legacy object-storage implementations that were provided via the trino-hdfs (Hadoop-based) file system layer, requiring catalogs to use the native S3/Azure/GCS file systems for object storage and leaving fs.hadoop.enabled applicable only to HDFS. Documentation is updated with migration guidance, and CI coverage is adjusted accordingly.
Changes:
- Delete legacy S3/Azure/GCS/IBM COS (and related security-mapping/config) code and tests from
lib/trino-hdfs. - Simplify HDFS filesystem manager/loader wiring so it no longer conditionally enables legacy object-storage modules.
- Update object storage and connector docs to reflect the breaking change and migration path; remove HDFS cloud-tests job for
trino-hdfs.
Reviewed changes
Copilot reviewed 88 out of 88 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping.json | Removed legacy S3 security-mapping test resource |
| lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-without-fallback.json | Removed legacy S3 security-mapping test resource |
| lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-with-fallback-to-cluster-default.json | Removed legacy S3 security-mapping test resource |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestUriBasedS3SecurityMappingsProvider.java | Removed legacy S3 security-mapping HTTP provider test |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemMinio.java | Removed legacy S3 Minio integration test |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemAwsS3.java | Removed legacy S3 AWS integration test |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingsParser.java | Removed legacy S3 security-mapping parser test |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingConfig.java | Removed legacy S3 security-mapping config test |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMapping.java | Removed legacy S3 security-mapping behavior tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3HadoopPaths.java | Removed S3-specific Hadoop path canonicalization tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3TypeConfig.java | Removed legacy Hive S3 filesystem-type config tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3Config.java | Removed legacy Hive S3 config tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/MockAmazonS3.java | Removed legacy S3 mock implementation used by tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/AbstractTestTrinoS3FileSystem.java | Removed legacy S3 filesystem test base |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/HdfsTestUtils.java | Removed legacy object-storage configuration initializers from shared test utilities |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/gcs/TestHiveGcsConfig.java | Removed legacy GCS config tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestServiceConfig.java | Removed legacy IBM COS service-config tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfigurationProvider.java | Removed legacy IBM COS per-bucket credential provider tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfig.java | Removed legacy IBM COS config tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestTrinoAzureConfigurationInitializer.java | Removed legacy Azure configuration initializer tests |
| lib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestHiveAzureConfig.java | Removed legacy Azure config tests |
| lib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemS3Mock.java | Removed HDFS-backed S3Mock tests for legacy object storage |
| lib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemManager.java | Updated to reflect that legacy S3 config properties are no longer configured/recognized |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/UriBasedS3SecurityMappingsProvider.java | Removed legacy S3 security-mapping HTTP provider |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3StorageClass.java | Removed legacy S3 storage-class enum |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SseType.java | Removed legacy S3 SSE type enum |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SignerType.java | Removed legacy S3 signer type enum |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3Protocol.java | Removed legacy S3 proxy protocol enum |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystemStats.java | Removed legacy S3 filesystem metrics/JMX stats |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3ConfigurationInitializer.java | Removed legacy S3 Hadoop configuration initializer |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3AclType.java | Removed legacy S3 canned ACL mapping enum |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsProvider.java | Removed legacy S3 security-mappings provider SPI |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsParser.java | Removed legacy S3 security-mappings JSON parser |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappings.java | Removed legacy S3 security-mappings model |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfigurationProvider.java | Removed legacy S3 dynamic configuration provider |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfig.java | Removed legacy S3 security-mapping config |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMapping.java | Removed legacy S3 security-mapping rule model |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3FileSystemType.java | Removed legacy S3 filesystem-type selection enum |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/RetryDriver.java | Removed legacy retry helper used by legacy S3 implementation |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3TypeConfig.java | Removed legacy Hive S3 type config |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Module.java | Removed legacy Hive S3 module wiring |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Config.java | Removed legacy Hive S3 config |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/ForS3SecurityMapping.java | Removed legacy Guice binding annotation for S3 security mapping |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/FileBasedS3SecurityMappingsProvider.java | Removed legacy file-based S3 security-mapping provider |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsSdkClientCoreStats.java | Removed legacy AWS SDK v1 client metrics collector |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsCurrentRegionHolder.java | Removed legacy AWS region metadata helper |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/OpenTelemetryAwareFileSystem.java | Removed legacy OpenTelemetry hook interface for Hadoop FS wrappers |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/MemoryAwareFileSystem.java | Removed legacy memory-aware Hadoop FS interface |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsModule.java | Removed optional binding related to legacy GCS support |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsEnvironment.java | Dropped legacy GCS/OpenTelemetry integration points from HDFS environment |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsModule.java | Removed legacy Hive GCS module wiring |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsConfig.java | Removed legacy Hive GCS config |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GoogleGcsConfigurationInitializer.java | Removed legacy Hadoop GCS configuration initializer |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsStorageFactory.java | Removed legacy GCS Storage factory used by Hadoop GCS connector |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsConfigurationProvider.java | Removed legacy GCS dynamic configuration provider |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAtomicOutputStream.java | Removed legacy GCS atomic createExclusive implementation |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAccessTokenProvider.java | Removed legacy GCS OAuth token provider |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/FileSystemWithBatchDelete.java | Removed legacy batch-delete extension interface |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/TrinoCosFileSystem.java | Removed legacy IBM COS Hadoop filesystem implementation |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/ServiceConfig.java | Removed legacy IBM COS service config loader |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosServiceConfig.java | Removed legacy IBM COS service config |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosModule.java | Removed legacy IBM COS module wiring |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosServiceConfigurationProvider.java | Removed legacy IBM COS dynamic configuration provider |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosConfigurationInitializer.java | Removed legacy IBM COS configuration initializer |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/TrinoAzureConfigurationInitializer.java | Removed legacy Azure Hadoop configuration initializer |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureModule.java | Removed legacy Azure module wiring |
| lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureConfig.java | Removed legacy Azure config |
| lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsOutputFile.java | Removed legacy GCS/MemoryAware output behaviors from Hadoop-backed output file |
| lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystemManager.java | Removed legacy object-storage modules from HDFS filesystem manager bootstrap |
| lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.java | Removed legacy scheme assumptions and batch-delete path for object storage |
| lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HadoopPaths.java | Simplified path creation after removing legacy S3 path-preservation hack |
| lib/trino-hdfs/src/main/java/com/google/cloud/hadoop/fs/gcs/TrinoGoogleHadoopFileSystemConfiguration.java | Removed legacy wrapper for shaded GCS connector configuration access |
| lib/trino-hdfs/pom.xml | Removed legacy AWS SDK v1/GCS connector and related test dependencies/profiles |
| lib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/HdfsFileSystemLoader.java | Updated reflective construction of HDFS manager after removing legacy toggles |
| lib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/HdfsClassLoader.java | Removed OpenTelemetry packages from override list since trino-hdfs no longer references them |
| lib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/FileSystemModule.java | Simplified loader initialization (no legacy object-storage enablement booleans) |
| docs/src/main/sphinx/object-storage/file-system-s3.md | Updated migration docs to reflect removal of legacy S3 filesystem |
| docs/src/main/sphinx/object-storage/file-system-gcs.md | Updated migration docs to reflect removal of legacy GCS filesystem |
| docs/src/main/sphinx/object-storage/file-system-azure.md | Updated migration docs to reflect removal of legacy Azure filesystem |
| docs/src/main/sphinx/object-storage.md | Updated fs.hadoop.enabled meaning and legacy support narrative |
| docs/src/main/sphinx/connector/lakehouse.md | Updated connector docs to clarify fs.hadoop.enabled is HDFS-only |
| docs/src/main/sphinx/connector/iceberg.md | Updated connector docs to clarify fs.hadoop.enabled is HDFS-only |
| docs/src/main/sphinx/connector/hudi.md | Updated connector docs to clarify fs.hadoop.enabled is HDFS-only |
| docs/src/main/sphinx/connector/hive.md | Updated connector docs to clarify fs.hadoop.enabled is HDFS-only |
| docs/src/main/sphinx/connector/delta-lake.md | Updated connector docs to clarify fs.hadoop.enabled is HDFS-only |
| .github/workflows/ci.yml | Removed trino-hdfs cloud-tests job/step from CI matrix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughThis pull request removes all legacy file system support for S3, Azure, Google Cloud Storage, and IBM COS from the Trino HDFS module. The changes eliminate deprecated configuration classes, initialization logic, file system implementations, security mapping infrastructure, and associated tests. Documentation is updated to remove references to legacy file system support and redirect users to native implementations. CI configuration is modified to remove cloud test execution that relied on legacy S3 support. Assessment against linked issues
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.java (1)
424-448: Consider adding "viewfs" to the explicit hierarchical check for consistency.The code now only explicitly handles "hdfs" as hierarchical, but
TrinoFileSystemCache.isHdfs()(inlib/trino-hdfs/src/main/java/io/trino/hdfs/TrinoFileSystemCache.java) treats both "hdfs" and "viewfs" as HDFS schemes. While the fallback probing logic will correctly identify ViewFS as hierarchical, adding an explicit check would be more consistent and avoid the probing overhead.💡 Optional: Add viewfs to explicit check
private boolean hierarchical(FileSystem fileSystem, Location rootLocation) { - if ("hdfs".equals(fileSystem.getScheme())) { + String scheme = fileSystem.getScheme(); + if ("hdfs".equals(scheme) || "viewfs".equals(scheme)) { return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.java` around lines 424 - 448, The hierarchical(FileSystem fileSystem, Location rootLocation) method only explicitly treats the "hdfs" scheme as hierarchical; update the initial scheme check to include "viewfs" as well (e.g., check if fileSystem.getScheme() equals "hdfs" or "viewfs") so ViewFS short-circuits to true like TrinoFileSystemCache.isHdfs(), avoiding the probe and cache write; make this change in the hierarchical method and leave the rest of the probing and hierarchicalFileSystemCache logic unchanged.lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsOutputFile.java (1)
54-58: UnusedmemoryContextparameter is misleading.The
memoryContextparameter is now completely ignored, which makes the API misleading to callers. While this aligns with the removal ofMemoryAwareFileSystem, callers likeLineFileWriterFactorystill pass a memory context expecting it to be used for memory tracking.Consider either:
- Documenting that the parameter is intentionally ignored (e.g., via a comment or deprecation)
- Using a simpler overload that doesn't accept the parameter
📝 Suggested documentation addition
`@Override` public OutputStream create(AggregatedMemoryContext memoryContext) throws IOException { + // memoryContext is intentionally unused - HDFS streams manage their own buffering return create(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsOutputFile.java` around lines 54 - 58, The create(AggregatedMemoryContext memoryContext) method in HdfsOutputFile currently ignores its memoryContext parameter which is misleading; update HdfsOutputFile so the overload is clearly intentional: mark create(AggregatedMemoryContext) as `@Deprecated`, add a Javadoc comment stating the memoryContext is intentionally ignored (and that callers should use create() / create(boolean)), and keep it delegating to create(false); alternatively, remove the overload and update callers such as LineFileWriterFactory to call create() or create(false) directly—choose one approach and apply it consistently so the API and callers reflect that memory tracking is not used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/src/main/sphinx/object-storage/file-system-s3.md`:
- Around line 293-295: Update the sentence that limits legacy S3 removal to
"Delta Lake, Hudi, or Iceberg" so it matches the migration scope: state that
legacy Amazon S3 support (configured via legacy hive.s3.* properties) has been
removed for any catalog that relied on those settings rather than only Delta
Lake/Hudi/Iceberg; in other words, replace or extend the phrase in the sentence
starting "Previous Trino releases included legacy Amazon S3 support..." to
mention "any catalog that relied on legacy hive.s3.* properties" or similar
wording and point readers to the migration section for details.
---
Nitpick comments:
In `@lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.java`:
- Around line 424-448: The hierarchical(FileSystem fileSystem, Location
rootLocation) method only explicitly treats the "hdfs" scheme as hierarchical;
update the initial scheme check to include "viewfs" as well (e.g., check if
fileSystem.getScheme() equals "hdfs" or "viewfs") so ViewFS short-circuits to
true like TrinoFileSystemCache.isHdfs(), avoiding the probe and cache write;
make this change in the hierarchical method and leave the rest of the probing
and hierarchicalFileSystemCache logic unchanged.
In `@lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsOutputFile.java`:
- Around line 54-58: The create(AggregatedMemoryContext memoryContext) method in
HdfsOutputFile currently ignores its memoryContext parameter which is
misleading; update HdfsOutputFile so the overload is clearly intentional: mark
create(AggregatedMemoryContext) as `@Deprecated`, add a Javadoc comment stating
the memoryContext is intentionally ignored (and that callers should use create()
/ create(boolean)), and keep it delegating to create(false); alternatively,
remove the overload and update callers such as LineFileWriterFactory to call
create() or create(false) directly—choose one approach and apply it consistently
so the API and callers reflect that memory tracking is not used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3a14dc00-f999-4dac-88aa-a7887add04ca
📒 Files selected for processing (88)
.github/workflows/ci.ymldocs/src/main/sphinx/connector/delta-lake.mddocs/src/main/sphinx/connector/hive.mddocs/src/main/sphinx/connector/hudi.mddocs/src/main/sphinx/connector/iceberg.mddocs/src/main/sphinx/connector/lakehouse.mddocs/src/main/sphinx/object-storage.mddocs/src/main/sphinx/object-storage/file-system-azure.mddocs/src/main/sphinx/object-storage/file-system-gcs.mddocs/src/main/sphinx/object-storage/file-system-s3.mdlib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/FileSystemModule.javalib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/HdfsClassLoader.javalib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/HdfsFileSystemLoader.javalib/trino-hdfs/pom.xmllib/trino-hdfs/src/main/java/com/google/cloud/hadoop/fs/gcs/TrinoGoogleHadoopFileSystemConfiguration.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HadoopPaths.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystemManager.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsOutputFile.javalib/trino-hdfs/src/main/java/io/trino/hdfs/FileSystemWithBatchDelete.javalib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsEnvironment.javalib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/MemoryAwareFileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/OpenTelemetryAwareFileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/azure/TrinoAzureConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosServiceConfigurationProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosServiceConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/ServiceConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/TrinoCosFileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAccessTokenProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAtomicOutputStream.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsConfigurationProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsStorageFactory.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GoogleGcsConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsCurrentRegionHolder.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsSdkClientCoreStats.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/FileBasedS3SecurityMappingsProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/ForS3SecurityMapping.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Config.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Module.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3TypeConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/RetryDriver.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3FileSystemType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMapping.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfigurationProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappings.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsParser.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3AclType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3ConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystemStats.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3Protocol.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SignerType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SseType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3StorageClass.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/UriBasedS3SecurityMappingsProvider.javalib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemManager.javalib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemS3Mock.javalib/trino-hdfs/src/test/java/io/trino/hdfs/HdfsTestUtils.javalib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestHiveAzureConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestTrinoAzureConfigurationInitializer.javalib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfigurationProvider.javalib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestServiceConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/gcs/TestHiveGcsConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/AbstractTestTrinoS3FileSystem.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/MockAmazonS3.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3Config.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3TypeConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3HadoopPaths.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMapping.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingsParser.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystem.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemAwsS3.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemMinio.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestUriBasedS3SecurityMappingsProvider.javalib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-with-fallback-to-cluster-default.jsonlib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-without-fallback.jsonlib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping.json
💤 Files with no reviewable changes (71)
- lib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/HdfsClassLoader.java
- lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-without-fallback.json
- lib/trino-hdfs/src/main/java/io/trino/hdfs/OpenTelemetryAwareFileSystem.java
- lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-with-fallback-to-cluster-default.json
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosConfigurationInitializer.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/FileSystemWithBatchDelete.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/MemoryAwareFileSystem.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsModule.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosModule.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestHiveAzureConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureModule.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/TrinoCosFileSystem.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/ForS3SecurityMapping.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3Protocol.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingsParser.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3FileSystemType.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3TypeConfig.java
- lib/trino-hdfs/src/main/java/com/google/cloud/hadoop/fs/gcs/TrinoGoogleHadoopFileSystemConfiguration.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3AclType.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SseType.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosServiceConfigurationProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsModule.java
- lib/trino-hdfs/pom.xml
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SignerType.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfigurationProvider.java
- .github/workflows/ci.yml
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3StorageClass.java
- lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping.json
- lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestServiceConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappings.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsParser.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestUriBasedS3SecurityMappingsProvider.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3Config.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosServiceConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsConfigurationProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAtomicOutputStream.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/MockAmazonS3.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemMinio.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3TypeConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAccessTokenProvider.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3HadoopPaths.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsCurrentRegionHolder.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/gcs/TestHiveGcsConfig.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3ConfigurationInitializer.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingConfig.java
- lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystemManager.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/TrinoAzureConfigurationInitializer.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemAwsS3.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/UriBasedS3SecurityMappingsProvider.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestTrinoAzureConfigurationInitializer.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsStorageFactory.java
- lib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemS3Mock.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfigurationProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsSdkClientCoreStats.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GoogleGcsConfigurationInitializer.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsConfig.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystem.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/AbstractTestTrinoS3FileSystem.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMapping.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMapping.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Config.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/RetryDriver.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/FileBasedS3SecurityMappingsProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystemStats.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/ServiceConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Module.java
3f128bd to
60e40d3
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.java`:
- Around line 136-144: The deleteFiles loop ignores the boolean return from
rawFileSystem.delete while deleteFile throws when delete() returns false; update
deleteFiles (and the loop using rawFileSystem.delete(path, false)) to check the
returned boolean and handle failures consistently: after calling
rawFileSystem.delete(path, false) capture the boolean result, and if false
create and record an IOException via
stats.getDeleteFileCalls().recordException(...) and rethrow the IOException
(matching deleteFile behavior), or if you choose lenient semantics adjust
deleteFile instead so both methods have the same documented contract; reference
the deleteFiles loop, deleteFile method, rawFileSystem.delete, and
stats.getDeleteFileCalls when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6e4a133-af2c-4756-9c9b-c85f12dd4269
📒 Files selected for processing (87)
.github/workflows/ci.ymldocs/src/main/sphinx/connector/delta-lake.mddocs/src/main/sphinx/connector/hive.mddocs/src/main/sphinx/connector/hudi.mddocs/src/main/sphinx/connector/iceberg.mddocs/src/main/sphinx/connector/lakehouse.mddocs/src/main/sphinx/object-storage.mddocs/src/main/sphinx/object-storage/file-system-azure.mddocs/src/main/sphinx/object-storage/file-system-gcs.mddocs/src/main/sphinx/object-storage/file-system-s3.mdlib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/FileSystemModule.javalib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/HdfsFileSystemLoader.javalib/trino-hdfs/pom.xmllib/trino-hdfs/src/main/java/com/google/cloud/hadoop/fs/gcs/TrinoGoogleHadoopFileSystemConfiguration.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HadoopPaths.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystemManager.javalib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsOutputFile.javalib/trino-hdfs/src/main/java/io/trino/hdfs/FileSystemWithBatchDelete.javalib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsEnvironment.javalib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/MemoryAwareFileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/OpenTelemetryAwareFileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/azure/TrinoAzureConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosServiceConfigurationProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosServiceConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/ServiceConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/cos/TrinoCosFileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAccessTokenProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAtomicOutputStream.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsConfigurationProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsStorageFactory.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GoogleGcsConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsModule.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsCurrentRegionHolder.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsSdkClientCoreStats.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/FileBasedS3SecurityMappingsProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/ForS3SecurityMapping.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Config.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Module.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3TypeConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/RetryDriver.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3FileSystemType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMapping.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfig.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfigurationProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappings.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsParser.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsProvider.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3AclType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3ConfigurationInitializer.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystemStats.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3Protocol.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SignerType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SseType.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3StorageClass.javalib/trino-hdfs/src/main/java/io/trino/hdfs/s3/UriBasedS3SecurityMappingsProvider.javalib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemManager.javalib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemS3Mock.javalib/trino-hdfs/src/test/java/io/trino/hdfs/HdfsTestUtils.javalib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestHiveAzureConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestTrinoAzureConfigurationInitializer.javalib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfigurationProvider.javalib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestServiceConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/gcs/TestHiveGcsConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/AbstractTestTrinoS3FileSystem.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/MockAmazonS3.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3Config.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3TypeConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3HadoopPaths.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMapping.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingConfig.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingsParser.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystem.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemAwsS3.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemMinio.javalib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestUriBasedS3SecurityMappingsProvider.javalib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-with-fallback-to-cluster-default.jsonlib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-without-fallback.jsonlib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping.json
💤 Files with no reviewable changes (69)
- lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-with-fallback-to-cluster-default.json
- lib/trino-hdfs/src/main/java/io/trino/hdfs/HdfsModule.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/ForS3SecurityMapping.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosConfigurationInitializer.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsProvider.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestUriBasedS3SecurityMappingsProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/OpenTelemetryAwareFileSystem.java
- lib/trino-hdfs/src/main/java/com/google/cloud/hadoop/fs/gcs/TrinoGoogleHadoopFileSystemConfiguration.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SignerType.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3FileSystemType.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3SseType.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsCurrentRegionHolder.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingsParser.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingsParser.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/TrinoCosFileSystem.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/CosServiceConfigurationProvider.java
- lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping.json
- lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystemManager.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/FileBasedS3SecurityMappingsProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3Protocol.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3TypeConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosServiceConfig.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestHiveAzureConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAtomicOutputStream.java
- .github/workflows/ci.yml
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappings.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsModule.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/gcs/TestHiveGcsConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3StorageClass.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemAwsS3.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureModule.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMappingConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3AclType.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/UriBasedS3SecurityMappingsProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsConfigurationProvider.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestHiveCosServiceConfigurationProvider.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/cos/TestServiceConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/MemoryAwareFileSystem.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/HiveCosModule.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsAccessTokenProvider.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/azure/TestTrinoAzureConfigurationInitializer.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystemStats.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GoogleGcsConfigurationInitializer.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestHiveS3Config.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3TypeConfig.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/MockAmazonS3.java
- lib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemS3Mock.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/HiveGcsConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/gcs/GcsStorageFactory.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3ConfigurationInitializer.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/TrinoAzureConfigurationInitializer.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3HadoopPaths.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/cos/ServiceConfig.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestS3SecurityMapping.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMappingConfigurationProvider.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Module.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/S3SecurityMapping.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/AwsSdkClientCoreStats.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystem.java
- lib/trino-hdfs/src/test/resources/io/trino/hdfs/s3/security-mapping-without-fallback.json
- lib/trino-hdfs/src/main/java/io/trino/hdfs/azure/HiveAzureConfig.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/RetryDriver.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/TestTrinoS3FileSystemMinio.java
- lib/trino-hdfs/src/test/java/io/trino/hdfs/s3/AbstractTestTrinoS3FileSystem.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/HiveS3Config.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/s3/TrinoS3FileSystem.java
- lib/trino-hdfs/src/main/java/io/trino/hdfs/FileSystemWithBatchDelete.java
✅ Files skipped from review due to trivial changes (4)
- docs/src/main/sphinx/connector/lakehouse.md
- docs/src/main/sphinx/connector/delta-lake.md
- docs/src/main/sphinx/connector/iceberg.md
- docs/src/main/sphinx/object-storage.md
🚧 Files skipped from review as they are similar to previous changes (10)
- docs/src/main/sphinx/connector/hudi.md
- docs/src/main/sphinx/connector/hive.md
- lib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/FileSystemModule.java
- lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HadoopPaths.java
- lib/trino-hdfs/src/test/java/io/trino/filesystem/hdfs/TestHdfsFileSystemManager.java
- docs/src/main/sphinx/object-storage/file-system-s3.md
- docs/src/main/sphinx/object-storage/file-system-gcs.md
- docs/src/main/sphinx/object-storage/file-system-azure.md
- lib/trino-filesystem-manager/src/main/java/io/trino/filesystem/manager/HdfsFileSystemLoader.java
- lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsOutputFile.java
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 87 out of 87 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lib/trino-hdfs/pom.xml:78
- Removing the legacy object-storage dependencies from
trino-hdfsmeansHdfsFileSystemFactorycan no longer be used to access object storage schemes (e.g.,s3://). There are still cloud tests that appear to rely onHdfsFileSystemFactoryto read from S3 (for example,plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.javaconstructs anHdfsFileSystemFactory, whileschemaPath()returns ans3://...location andisFileSorted()reads via that factory). This is likely to fail at runtime after these dependency removals. Migrate those tests/usages to the native object storage file system factories (e.g.,trino-filesystem-s3) or otherwise avoidHdfsFileSystemFactoryfors3://paths.
<dependencies>
<dependency>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_annotations</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
<dependency>
<groupId>com.google.inject</groupId>
<artifactId>guice</artifactId>
<classifier>classes</classifier>
</dependency>
<dependency>
<groupId>io.airlift</groupId>
<artifactId>bootstrap</artifactId>
</dependency>
<dependency>
<groupId>io.airlift</groupId>
<artifactId>configuration</artifactId>
</dependency>
<dependency>
<groupId>io.airlift</groupId>
<artifactId>log</artifactId>
</dependency>
<dependency>
<groupId>io.airlift</groupId>
<artifactId>stats</artifactId>
</dependency>
<dependency>
<groupId>io.airlift</groupId>
<artifactId>units</artifactId>
</dependency>
<dependency>
<groupId>io.trino</groupId>
<artifactId>trino-filesystem</artifactId>
</dependency>
<dependency>
<groupId>io.trino</groupId>
<artifactId>trino-memory-context</artifactId>
</dependency>
<dependency>
<groupId>io.trino</groupId>
<artifactId>trino-plugin-toolkit</artifactId>
</dependency>
<dependency>
<groupId>io.trino.hadoop</groupId>
<artifactId>hadoop-apache</artifactId>
</dependency>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
60e40d3 to
51dec0c
Compare
51dec0c to
2c5615c
Compare
f949878 to
fe61151
Compare
Description
Remove legacy object storage support for Azure Storage, Google Cloud Storage, IBM Cloud Object Storage, S3, and S3-compatible systems. Require catalogs to use the native Azure, GCS, or S3 file systems for object storage, keep
fs.hadoop.enabledonly for HDFS, and document the migration path for existing configurations.Additional context and related issues
Release notes
(x) Release notes are required, with the following suggested text: