Skip to content

fix: ModelRegistryClient ignored explicit URL ports - #763

Open
a-reda-ok wants to merge 1 commit into
kubeflow:mainfrom
a-reda-ok:fix-url-ports
Open

fix: ModelRegistryClient ignored explicit URL ports#763
a-reda-ok wants to merge 1 commit into
kubeflow:mainfrom
a-reda-ok:fix-url-ports

Conversation

@a-reda-ok

Copy link
Copy Markdown

What this PR does / why we need it:

Fixes ModelRegistryClient port inference so URLs with an explicit port (for example, https://example.org:456 or http://example.org:456) correctly use that port when port is not passed as an argument.

Previously, initialization only inferred port from scheme (443 for HTTPS, 8080 for HTTP), which silently ignored explicit URL ports and could route clients to the wrong endpoint. The fix parses base_url with urlsplit and prefers parsed_url.port before falling back to scheme defaults. This preserves existing behavior for URLs without explicit ports while making documented explicit-port behavior actually work.

Regression coverage was updated in model_registry_client_test.py so both explicit HTTP/HTTPS port cases now assert port=456, validating the corrected behavior.

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):

Fixes #

Checklist:

  • Docs included if any changes are user facing

…ot passed explicitly.

Signed-off-by: Reda Boussaadi <redaredaboussaadi@gmail.com>
Copilot AI balanced review requested due to automatic review settings August 17, 2026 00:30
@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign electronic-waste for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow
google-oss-prow Bot requested review from fege and szaher August 17, 2026 00:30

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

🎉 Welcome to the Kubeflow SDK! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

  • If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards
  • Our team will review your PR soon! cc @kubeflow/kubeflow-sdk-team

Join the community:

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

@a-reda-ok a-reda-ok changed the title Fix: ModelRegistryClient ignored explicit URL ports fix: ModelRegistryClient ignored explicit URL ports Aug 17, 2026
@a-reda-ok
a-reda-ok requested a balanced review from Copilot August 17, 2026 17:12

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants