wcmolin commented on issue #18267: URL: https://github.com/apache/dolphinscheduler/issues/18267#issuecomment-4473395784
I agree with @leocook's analysis and lean toward not introducing a separate plugin for now. In my view, this is not a new storage type, but an endpoint/region compatibility issue within the S3 protocol implementation. AWS SDK's endpoint configuration is the documented mechanism for overriding the S3 endpoint, and the region is used as the signing region string. When a custom endpoint is configured, DolphinScheduler can pass the configured region string directly to `EndpointConfiguration`; validating it with `Regions.fromName()` is stricter than what the SDK requires and breaks S3-compatible storage such as MinIO. From the current code structure, both S3 resource storage and remote logging go through the same `AmazonS3ClientFactory`, so handling this in the factory keeps the change small and centralized: use the raw region string when `endpoint` is configured, and only use `Regions.fromName()` when no custom endpoint is configured. `S3StorageOperator` already reuses the same factory, so this avoids near-duplicate implementations, two configuration entries, and duplicated future bug fixes. I understand @ruanwenjun's concern that more compatibility differences may appear in the future. Maybe a reasonable boundary is: as long as the behavior is still S3 API based and can be handled by AWS SDK configuration, we keep it in the existing S3 plugin; if we later find substantial differences in authentication model, object semantics, listing/multipart behavior, or other SDK-level behavior that cannot be unified cleanly, then introducing a separate plugin would be justified. This keeps the current fix minimal while still leaving room to split it later if real divergence appears. -- 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]
