Skip to content

Conversation

MSeal
Copy link
Contributor

@MSeal MSeal commented Oct 11, 2025

Fixes #2098

Converts the arguments to keyword args to avoid ordering issues with class definition.

Checklist

  • Contains customer facing changes? Including API/behavior changes
  • Did you add sufficient unit test and/or integration test coverage for this PR?
    • If not, please explain why it is not required

References

JIRA:

Test & Review

Open questions / Follow-ups

@MSeal MSeal requested a review from a team as a code owner October 11, 2025 00:05
@Copilot Copilot AI review requested due to automatic review settings October 11, 2025 00:05
@MSeal MSeal requested a review from a team as a code owner October 11, 2025 00:05
Copy link

@Copilot 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 an invalid argument configuration in the RegisteredSchema constructor by converting positional arguments to keyword arguments, addressing an argument ordering issue that was causing errors.

  • Converted RegisteredSchema constructor calls from positional to keyword arguments
  • Added a test case to verify the register_schema_full_response method works correctly
  • Removed a trailing comma in a RegisteredSchema constructor call

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/confluent_kafka/schema_registry/_async/schema_registry_client.py Fixed RegisteredSchema constructor calls to use keyword arguments instead of positional arguments
tests/schema_registry/_async/test_api_client.py Added test case for register_schema_full_response method to ensure proper functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@sonarqube-confluent

This comment has been minimized.

@pbabics
Copy link

pbabics commented Oct 13, 2025

@MSeal
Copy link
Contributor Author

MSeal commented Oct 13, 2025

@pbabics thanks, I forgot to run the unasync command to update the generated code. The semaphore build also referenced this change was missing.

@sonarqube-confluent

This comment has been minimized.

Copy link
Member

@fangnx fangnx left a comment

Choose a reason for hiding this comment

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

LGTM other than the unit test comments. Thank you for the quick fix!

sr = AsyncSchemaRegistryClient(conf)
schema = Schema(load_avsc('basic_schema.avsc'), schema_type='AVRO')

await sr.register_schema('test-key', schema)
Copy link
Member

Choose a reason for hiding this comment

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

Should this line be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's intended to force the cache load of an already registered schema for the test

sr = SchemaRegistryClient(conf)
schema = Schema(load_avsc('basic_schema.avsc'), schema_type='AVRO')

sr.register_schema('test-key', schema)
Copy link
Member

Choose a reason for hiding this comment

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

Same

@MSeal MSeal enabled auto-merge (squash) October 15, 2025 15:33
@MSeal MSeal merged commit 7a6f0de into master Oct 15, 2025
2 checks passed
@MSeal MSeal deleted the mseal/fix-register-call branch October 15, 2025 16:11
@sonarqube-confluent
Copy link

Passed

Analysis Details

2 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 2 Code Smells

Coverage and Duplications

  • Coverage 100.00% Coverage (66.90% Estimated after merge)
  • Duplications No duplication information (4.90% Estimated after merge)

Project ID: confluent-kafka-python

View in SonarQube

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.

Schema-Registry client badly formed hexadecimal UUID string

3 participants