Skip to content

Comments

[test](p2) allow protocol prefix in S3 endpoint format in test_broker_load#53525

Merged
liaoxin01 merged 3 commits intoapache:masterfrom
kaijchen:fix_test_broker_load
Jul 21, 2025
Merged

[test](p2) allow protocol prefix in S3 endpoint format in test_broker_load#53525
liaoxin01 merged 3 commits intoapache:masterfrom
kaijchen:fix_test_broker_load

Conversation

@kaijchen
Copy link
Member

@kaijchen kaijchen commented Jul 18, 2025 •
edited
Loading

What problem does this PR solve?

Related PR: #51529

Problem Summary:

PR #51529 introduced a change that adds http:// to S3 endpoints if no protocol is specified.
For example, cluster:${s3Endpoint} could become cluster:http://${s3Endpoint}.
This PR updates the test logic to accept S3 endpoints with extra protocol prefix .

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@kaijchen
Copy link
Member Author

kaijchen commented Jul 18, 2025

run buildall

@kaijchen kaijchen changed the title [test](p2) allow flexible S3 endpoint format in test_broker_load.groovy [test](p2) allow flexible S3 endpoint format in test_broker_load Jul 18, 2025
@kaijchen kaijchen mentioned this pull request Jul 18, 2025
Merged
@kaijchen kaijchen changed the title [test](p2) allow flexible S3 endpoint format in test_broker_load [test](p2) allow protocol prefix in S3 endpoint format in test_broker_load Jul 18, 2025
dataroaring pushed a commit that referenced this pull request Jul 21, 2025
@kaijchen
[test](p2) allow protocol prefix in S3 endpoint format in test_broker…
a689086 
…_load (#53525) (#53530)

cherry-pick #53525
dataroaring
dataroaring approved these changes Jul 21, 2025
View reviewed changes
Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 21, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 21, 2025

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 21, 2025

PR approved by anyone and no changes requested.

liaoxin01
liaoxin01 approved these changes Jul 21, 2025
View reviewed changes
Copy link
Contributor

@liaoxin01 liaoxin01 left a comment

Choose a reason for hiding this comment

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

LGTM

@liaoxin01 liaoxin01 merged commit 42834ca into apache:master Jul 21, 2025
32 of 33 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 21, 2025
@kaijchen
[test](p2) allow protocol prefix in S3 endpoint format in test_broker…
a761ddd 
…_load (#53525)

Related PR: #51529

Problem Summary:

PR #51529 introduced a change that adds `http://` to S3 endpoints if no
protocol is specified.
For example, `cluster:${s3Endpoint}` could become
`cluster:http://${s3Endpoint}`.
This PR updates the test logic to accept S3 endpoints with extra
protocol prefix .
@github-actions github-actions bot mentioned this pull request Jul 21, 2025
Merged
morrySnow pushed a commit that referenced this pull request Jul 22, 2025
@github-actions @kaijchen
branch-3.1: [test](p2) allow protocol prefix in S3 endpoint format in…
b9a7183 
… test_broker_load #53525 (#53615)

Cherry-picked from #53525

Co-authored-by: Kaijie Chen <chenkaijie@selectdb.com>
@gavinchou gavinchou mentioned this pull request Aug 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liaoxin01 liaoxin01 liaoxin01 approved these changes

@dataroaring dataroaring dataroaring approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/3.0.7-merged dev/3.1.0-merged p0_test reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kaijchen @liaoxin01 @dataroaring @morrySnow