Skip to content

fix : UpstreamCheckUtils.checkUrl param add protocol. - #6252

Open
BraveheartStone wants to merge 12 commits into
apache:masterfrom
BraveheartStone:fix_upstream_check_utils
Open

BraveheartStone wants to merge 12 commits into
apache:masterfrom
BraveheartStone:fix_upstream_check_utils

Conversation

@BraveheartStone

Copy link
Copy Markdown
Contributor

If the protocol parameter is not added, when shenyu.upstreamCheck.enabled=true is configured in shenyu-bootstrap, the port numbers checked in the UpstreamCheckUtils.checkUrl method will all be port 80. The host using the HTTPS protocol will not be checked on port 443, resulting in the health check failing continuously.

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

@Aias00

Aias00 commented Jan 16, 2026

Copy link
Copy Markdown
Contributor

@BraveheartStone hi, plx fix ci

@Aias00 Aias00 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.

REQUEST_CHANGES (PMC Aias00): this PR has merge conflicts (CONFLICTING) and cannot be merged. Please rebase onto the latest master and resolve the conflicts, then request a re-review.

@Aias00

Aias00 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Re-check: this branch is still CONFLICTING with master, and the grpc e2e case plus build (shenyu-integrated-test-grpc) are red. Please rebase onto the latest master and re-run CI.

@Aias00

Aias00 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Hi @BraveheartStone, this PR has been in Changes Requested since my earlier review and there has been no follow-up push addressing the requested fixes (last commit 2025-12-05).

To keep the review queue healthy, please either:

  • (a) push the requested changes and request a re-review, or
  • (b) rebase onto the latest master if it has diverged.

If there are no plans to continue, we may close it for staleness — happy to reopen if you pick it up later. Thanks!

@Aias00

Aias00 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Hi @BraveheartStone, this PR currently has merge conflicts with the master base branch, so CI cannot validate it against the latest code. Please rebase/merge the latest master and resolve the conflicts, then push the updated branch. Once the conflicts are cleared, CI will re-run automatically and we can continue the review. Thanks!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants