Skip to content

[feat](cloud) S3 storage vault support property use_path_style - #43060

Merged
dataroaring merged 1 commit into
apache:masterfrom
swjtu-zhanglei:master-fix1
Nov 6, 2024
Merged

[feat](cloud) S3 storage vault support property use_path_style#43060
dataroaring merged 1 commit into
apache:masterfrom
swjtu-zhanglei:master-fix1

Conversation

@swjtu-zhanglei

@swjtu-zhangleiswjtu-zhanglei commented Nov 1, 2024

Copy link
Copy Markdown
Contributor

example

 CREATE STORAGE VAULT IF NOT EXISTS demo_vault
PROPERTIES (
"type"="S3",
...
"use_path_style" = "true"
);

doc apache/doris-website#1344

@doris-robot

Copy link
Copy Markdown

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 37.82% (9820/25968)
Line Coverage: 28.98% (81577/281502)
Region Coverage: 28.26% (42144/149105)
Branch Coverage: 24.83% (21374/86070)
Coverage Report: http://coverage.selectdb-in.cc/coverage/261f97943bf7764d120d19caa554670d41aaff37_261f97943bf7764d120d19caa554670d41aaff37/report/index.html

dataroaring
dataroaring previously approved these changes Nov 1, 2024

@dataroaringdataroaring left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Nov 1, 2024
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

builder.setProvider(Provider.valueOf(properties.get(S3Properties.PROVIDER).toUpperCase()));
}

if (properties.containsKey(PropertyConverter.USE_PATH_STYLE)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be modified by alter vault stmt?

@swjtu-zhangleiswjtu-zhangleiNov 4, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be modified by alter vault stmt?

done

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actionsgithub-actionsBot removed the approved Indicates a PR has been approved by one committer. label Nov 4, 2024
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 37.80% (9819/25974)
Line Coverage: 28.98% (81660/281810)
Region Coverage: 28.21% (42094/149194)
Branch Coverage: 24.80% (21361/86140)
Coverage Report: http://coverage.selectdb-in.cc/coverage/6d7486e2d4b0ffca6842b076495c7a9cc0f0b5e6_6d7486e2d4b0ffca6842b076495c7a9cc0f0b5e6/report/index.html

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 37.81% (9822/25975)
Line Coverage: 28.99% (81711/281818)
Region Coverage: 28.23% (42121/149203)
Branch Coverage: 24.81% (21375/86144)
Coverage Report: http://coverage.selectdb-in.cc/coverage/ac77d1b29afb175eb61f09e22750a1158dd9fab2_ac77d1b29afb175eb61f09e22750a1158dd9fab2/report/index.html

String value = properties.get(PropertyConverter.USE_PATH_STYLE);
Preconditions.checkArgument(!Strings.isNullOrEmpty(value), "use_path_style cannot be empty");
Preconditions.checkArgument(value.equalsIgnoreCase("true")
|| value.equalsIgnoreCase("false"), "Invalid use_path_style value: %s", value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Invalid use_path_style value: %s, only 'true' or 'false' is acceptable"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Invalid use_path_style value: %s, only 'true' or 'false' is acceptable"

resolved

Comment on lines +344 to +349
if (properties.get(PropertyConverter.USE_PATH_STYLE).equalsIgnoreCase("true")) {
builder.setUsePathStyle(true);
} else {
builder.setUsePathStyle(false);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (properties.get(PropertyConverter.USE_PATH_STYLE).equalsIgnoreCase("true")) {
builder.setUsePathStyle(true);
} else {
builder.setUsePathStyle(false);
}
builder.setUsePathStyle(value.equalsIgnoreCase("true"));

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe alter set use_path_style from true to false

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

```
CREATE STORAGE VAULT IF NOT EXISTS demo_vault
PROPERTIES (
"type"="S3",
...
"use_path_style" = "true"
);
```
@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 37.86% (9837/25982)
Line Coverage: 29.03% (81810/281850)
Region Coverage: 28.27% (42167/149164)
Branch Coverage: 24.86% (21402/86100)
Coverage Report: http://coverage.selectdb-in.cc/coverage/b5f9d28d0a63ad0fa6f863add6088fa625a2d2d2_b5f9d28d0a63ad0fa6f863add6088fa625a2d2d2/report/index.html

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run p0

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run cloud_p0

@swjtu-zhanglei

Copy link
Copy Markdown
ContributorAuthor

run p0

@dataroaringdataroaring left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dataroaring
dataroaring merged commit 4a7b504 into apache:masterNov 6, 2024
github-actionsBot pushed a commit that referenced this pull request Nov 6, 2024
)
example ```
CREATE STORAGE VAULT IF NOT EXISTS demo_vault
PROPERTIES (
"type"="S3",
...
"use_path_style" = "true"
);
```
swjtu-zhanglei added a commit to swjtu-zhanglei/incubator-doris that referenced this pull request Nov 6, 2024
…che#43060)
example ```
CREATE STORAGE VAULT IF NOT EXISTS demo_vault
PROPERTIES (
"type"="S3",
...
"use_path_style" = "true"
);
```
dataroaring pushed a commit that referenced this pull request Nov 7, 2024
…_style` (#43330)
Cherry-picked from #43060
Co-authored-by: Lei Zhang <27994433+SWJTU-ZhangLei@users.noreply.github.com>
gavinchou pushed a commit that referenced this pull request Nov 8, 2024
) (#43343)
example ```
CREATE STORAGE VAULT IF NOT EXISTS demo_vault
PROPERTIES (
"type"="S3",
...
"use_path_style" = "true"
);
```
@gavinchougavinchou added cir and removed approved Indicates a PR has been approved by one committer. labels Nov 11, 2024
@gavinchougavinchou mentioned this pull request Nov 26, 2024
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Apr 24, 2026
…che#43060)
example ```
CREATE STORAGE VAULT IF NOT EXISTS demo_vault
PROPERTIES (
"type"="S3",
...
"use_path_style" = "true"
);
```
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@swjtu-zhanglei@doris-robot@gavinchou@Yukang-Lian@dataroaring