-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[chore](refactor-params)remove old properties #56163
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
[chore](refactor-params)remove old properties #56163
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-H: Total hot run time: 34967 ms |
TPC-DS: Total hot run time: 185907 ms |
ClickBench: Total hot run time: 29.93 s |
|
run buildall |
TPC-H: Total hot run time: 34517 ms |
TPC-DS: Total hot run time: 186958 ms |
ClickBench: Total hot run time: 30.41 s |
FE UT Coverage ReportIncrement line coverage |
|
run compile |
| private static final ExecutorService ALTER_BE_SYNC_THREAD_POOL = Executors.newFixedThreadPool(1); | ||
| private final SystemInfoService systemInfoService; | ||
|
|
||
| public static final String S3_ROOT_PATH = "s3.root.path"; |
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.
duplicate in S3StorageVault.java.
I think we can move it to S3Properties?
| return alterObjVaultBuilder; | ||
| } | ||
|
|
||
| private static Cloud.ObjectStoreInfoPB.Builder getObjStoreInfoPB(Map<String, String> properties) { |
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.
I think this method should be moved to S3Properties?
- properties -> S3StorageProperties
- S3StorageProperties -> ObjectStoreInfoPB
| S3Properties.SECRET_KEY, | ||
| S3Properties.Env.SECRET_KEY, | ||
| MCProperties.SECRET_KEY)); | ||
| SENSITIVE_KEY.addAll(S3Properties.SENSITIVE_KEYS); |
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.
Use method like SENSITIVE_KEY.addAll(ConnectorPropertiesUtils.getSensitiveKeys(OBSProperties.class));
| @Getter | ||
| protected String forceParsingByStandardUrl = "false"; | ||
|
|
||
| public static final String ENDPOINT = "azure.endpoint"; |
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.
These 4 strings are not used, can be removed
| return ret; | ||
| } | ||
|
|
||
| private static TS3StorageParam getS3TStorageParam(Map<String, String> properties) { |
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.
Need to refactor this later.
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-H: Total hot run time: 34872 ms |
TPC-DS: Total hot run time: 188862 ms |
ClickBench: Total hot run time: 29.15 s |
FE Regression Coverage ReportIncrement line coverage |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
remove oldcode (cherry picked from commit 87f8248)
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)