Skip to content

Conversation

@pko-trackunit
Copy link

Fixed few mismached URLs and missing default value for a parameter.

@pko-trackunit pko-trackunit requested a review from a team as a code owner June 26, 2023 13:51
@pko-trackunit
Copy link
Author

Also, it feels like lookup_schema() method should not be using an endpoint that "Register a new schema under the specified subject". Unless I am misunderstanding what "lookup" means.

@cla-assistant
Copy link

cla-assistant bot commented Aug 15, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@rayokota rayokota added the component:schema-registry Any schema registry related isues rather than kafka isolated ones label Jul 13, 2024
@fangnx
Copy link
Member

fangnx commented Jul 28, 2025

Hi @pko-trackunit, thanks for raising the PR. I double checked and the two doc URLs you fixed and the one for lookup_schema() (should be https://docs.confluent.io/platform/current/schema-registry/develop/api.html#post--subjects-(string- subject)) are indeed incorrect

Also, it feels like lookup_schema() method should not be using an endpoint that "Register a new schema under the specified subject". Unless I am misunderstanding what "lookup" means.

See Also:
`GET subjects API Reference <https://docs.confluent.io/current/schema-registry/develop/api.html#get--subjects-(string-%20subject)-versions>`_
`GET subjects API Reference <https://docs.confluent.io/platform/current/schema-registry/develop/api.html#get--subjects>`_
Copy link
Member

Choose a reason for hiding this comment

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

Let's omit the /platform part (which will be added during redirection) of the URl to stay consistent with the rest of URLs in docs

Copilot AI review requested due to automatic review settings July 29, 2025 15:18

This comment was marked as outdated.

@confluent-cla-assistant
Copy link

All contributors need to sign the Contributor License Agreement here before this PR can be approved.
❌ pko-trackunit
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@fangnx fangnx requested review from a team and MSeal as code owners July 29, 2025 15:51
@fangnx fangnx requested a review from Copilot July 29, 2025 15:52

This comment was marked as outdated.

@fangnx fangnx requested a review from Copilot July 29, 2025 16:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes documentation issues in SchemaRegistryClient by correcting mismatched API reference URLs and adding a missing default value for the version parameter.

  • Corrected four API reference URLs in docstrings to point to the correct endpoints
  • Added default value "latest" to the version parameter in get_version method

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/confluent_kafka/schema_registry/_sync/schema_registry_client.py Fixed API reference URLs in docstrings and added default value for version parameter
src/confluent_kafka/schema_registry/_async/schema_registry_client.py Same fixes as sync version - corrected URLs and added version parameter default

def get_version(
self, subject_name: str, version: int,
self, subject_name: str, version: Union[int, str] = "latest",
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

Adding a default value to an existing parameter is a breaking change for callers using positional arguments. Consider whether this change maintains backward compatibility with existing code that may call get_version(subject, version, deleted=True).

Suggested change
self, subject_name: str, version: Union[int, str] = "latest",
self, subject_name: str, version: Union[int, str] = "latest", *,
Copilot uses AI. Check for mistakes.
@rayokota
Copy link
Member

/sem-approve

@fangnx fangnx self-requested a review August 1, 2025 17:36
@fangnx fangnx mentioned this pull request Aug 1, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:schema-registry Any schema registry related isues rather than kafka isolated ones

3 participants