CalvinKirs commented on PR #65834: URL: https://github.com/apache/doris/pull/65834#issuecomment-5031259581
Thanks for the review. Addressed in the amended commit: **[P1] build.sh does not build the three new dialect modules** — Fixed. The FE `modules` selection loop (build.sh:720) only listed the pre-existing filesystem modules, so with `mvn package -pl ... -am` the new `gcs`/`minio`/`ozone` modules were never built while the deploy loop (build.sh:1051) still tried to unpack them — hence the missing `doris-fe-filesystem-gcs.zip`. Added `s3-base s3 gcs minio ozone` to the build loop so the build and deploy lists agree. Verified with a parallel (`-T`) clean package: all four `doris-fe-filesystem-*.zip` are produced and `s3-base` is bundled as a real jar in each plugin's `lib/`. **[P1] Ozone canonical region key not bound** — Fixed. `OzoneFileSystemProperties` accepted `AWS_*` aliases on every field except region, so a canonical `AWS_REGION` fell back to the `us-east-1` default. Added `AWS_REGION` to the region alias list (matching the MinIO sibling) and strengthened the test to bind a non-default `eu-west-2` and assert it reaches `toS3CompatibleKv()`. **[P1] Explicit DEFAULT credential mode overwritten with ANONYMOUS** — Fixed. `AbstractDelegatingS3Properties.toS3CompatibleKv()` unconditionally stamped `ANONYMOUS` when no static keys were present, silently downgrading an explicitly requested `DEFAULT`. It now preserves a requested provider type (validation already restricts it to `DEFAULT`/`ANONYMOUS`) and synthesizes `ANONYMOUS` only when none was requested. Added a test asserting `DEFAULT` survives. **[P1] Legacy MinIO/Ozone lose dialect identity through conversion** — Intentional scope, documented rather than changed. `StoragePropertiesConverter` rewrites a legacy `StorageProperties` to canonical `AWS_*` and only GCS carries a durable `provider=GCP`; legacy `MinioProperties`/`OzoneProperties` stamp no provider, so their converted maps are served by the generic S3 provider — which is exactly today's behavior (no dedicated MinIO/Ozone plugin existed before this PR). MinIO/Ozone are S3-compatible and the converter has already resolved their aliases and defaults into the canonical map, so the generic S3 client serves them identically; no behavior change and no regression. Dedicated routing applies to the raw/explicit path. Preserving a signal across conversion would require an fe-core change, which this PR deliberately avoids (fe-core net diff is zero). The PR description now states this explicitly. -- 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]
