-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[feat](ObjectStorage)Relax endpoint validation for private object storage #56579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…rage Body: vbnet Copy code Support private deployments by removing strict endpoint checks. If the endpoint is non-standard, region cannot be inferred automatically, so users must provide the region explicitly.
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
ClickBench: Total hot run time: 30.35 s |
|
run buildall |
ClickBench: Total hot run time: 30.32 s |
FE UT Coverage ReportIncrement line coverage |
|
run buildall |
|
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR relaxes endpoint validation for object storage systems to support private deployments. The change removes strict endpoint pattern matching and instead requires users to explicitly provide the region when using non-standard endpoints.
- Removes strict endpoint validation logic that was blocking private deployments
- Updates error handling to require region specification instead of endpoint validation
- Modifies test cases to reflect the new validation behavior
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test_domain_connection_and_ak_sk_correction.groovy | Updates error message assertion for connection failures |
| S3PropertiesTest.java | Removes endpoint validation tests and adds region requirement tests |
| OSSPropertiesTest.java | Changes endpoint validation to region requirement validation |
| OBSPropertyTest.java | Updates error message from endpoint validation to region requirement |
| COSPropertiesTest.java | Removes endpoint validation assertion |
| IcebergS3TablesMetaStorePropertiesTest.java | Updates test to check for region requirement earlier in validation |
| S3Properties.java | Overrides endpoint check requirement to disable validation |
| OSSProperties.java | Updates endpoint handling logic for non-standard endpoints |
| AbstractS3CompatibleProperties.java | Replaces endpoint validation with region requirement checks |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...c/main/java/org/apache/doris/datasource/property/storage/AbstractS3CompatibleProperties.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSProperties.java
Show resolved
Hide resolved
ClickBench: Total hot run time: 30.39 s |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
| throw new IllegalArgumentException("Both the access key and the secret key must be set."); | ||
| } | ||
| if (StringUtils.isBlank(getRegion())) { | ||
| throw new IllegalArgumentException("Region must be set."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
endpoint must be stand
...c/main/java/org/apache/doris/datasource/property/storage/AbstractS3CompatibleProperties.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSProperties.java
Outdated
Show resolved
Hide resolved
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
TPC-DS: Total hot run time: 190336 ms |
ClickBench: Total hot run time: 30.53 s |
|
run buildall |
TPC-DS: Total hot run time: 190611 ms |
ClickBench: Total hot run time: 30.7 s |
|
PR approved by at least one committer and no changes requested. |
…rage (#56579) … ### Support private deployments by removing strict endpoint checks. If the endpoint is non-standard, region cannot be inferred automatically, so users must provide the region explicitly.
…rage (#56579) … ### Support private deployments by removing strict endpoint checks. If the endpoint is non-standard, region cannot be inferred automatically, so users must provide the region explicitly.
apache#58572) Related PR: #apache#56579 Problem Summary: cuz we add the check : `Pattern STANDARD_ENDPOINT_PATTERN = Pattern .compile("^(?:https?://)?(?:s3\\.)?oss-([a-z0-9-]+?)(?:-internal)?\\.aliyuncs\\.com$");` the case will be invalid
…
Support private deployments by removing strict endpoint checks. If the endpoint is non-standard, region cannot be inferred automatically, so users must provide the region explicitly.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)