Skip to content

Refactor for async support #1971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

rohitsanj
Copy link

@rohitsanj rohitsanj commented Apr 17, 2025

What

This PR refactors the schema registry modules across a newly introduced _sync package, as well as the common package. The PR that introduces the _async package is stacked onto this one, see #1965.

To illustrate the changes made, let's take src/confluent_kafka/schema_registry/schema_registry_client.py for example:

  • Some of the common functionality was refactored out into src/confluent_kafka/schema_registry/common/schema_registry_client.py -- this package is intended to house functionality that is common to both the sync and async implementations.
  • The sync implementations of SchemaRegistryClient and its dependent classes were moved into src/confluent_kafka/schema_registry/_sync/schema_registry_client.py
  • The original src/schema_registry/schema_registry_client.py was refactored to only contain imports from the common.schema_registry_client and _sync.schema_registry_client modules. This is done so that users relying on imports directly from confluent_kafka.schema_registry.schema_registry_client don't encounter any breaking/missing imports.

The above treatment was applied to: schema_registry_client.py (as demonstrated), avro.py, json_schema.py, protobuf.py and serde.py.

Other changes:

  • The corresponding test files have also been moved into a new subpackage called _sync (with minimal changes).

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: PR only contains refactorings, no functionality has been added or removed.

References

JIRA:

Test & Review

Open questions / Follow-ups

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

…support

	src/confluent_kafka/schema_registry/avro.py (change applied to src/confluent_kafka/schema_registry/_sync/avro.py)
@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent
Copy link

Failed

  • 72.50% Coverage on New Code (is less than 80.00%)

Analysis Details

46 Issues

  • Bug 0 Bugs
  • Vulnerability 1 Vulnerability
  • Code Smell 45 Code Smells

Coverage and Duplications

  • Coverage 72.50% Coverage (62.50% Estimated after merge)
  • Duplications No duplication information (1.00% Estimated after merge)

Project ID: confluent-kafka-python

View in SonarQube

Comment on lines -80 to +81
with (patch("confluent_kafka.schema_registry.schema_registry_client.time.sleep") as mock_sleep,
patch("confluent_kafka.schema_registry.schema_registry_client.full_jitter") as mock_jitter):
with (patch("confluent_kafka.schema_registry._sync.schema_registry_client.time.sleep") as mock_sleep,
patch("confluent_kafka.schema_registry._sync.schema_registry_client.full_jitter") as mock_jitter):
Copy link
Author

Choose a reason for hiding this comment

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

unittest.mock.patch needs to be used directly with the module that defines the function to be patched, hence this change.

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.

1 participant