Skip to content
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

refactor: consolidate schema version resolution and validation logic #885

Merged
merged 13 commits into from
Jun 5, 2024

Conversation

nosahama
Copy link
Contributor

@nosahama nosahama commented May 29, 2024

About this change - What it does

Add support for -1 also as the latest schema version.

We add a class object to manage the schema version logic and refactor parts of the code to use this new schema version manager object to resolve the version and also perform validation, allowing us to handle the -1 as latest schema and other related validations without duplicating logic.

It was also drawn to my attention that we have to be backwards compatible with python-3.8 and above, thus some type hints still use earlier syntax and some are using newer ones, i faced this errors during the CI tests, thus test type hints are backwards compatible from python-3.8.

References:

Why this way

We want to keep the version related logic simple and dry.

- we add a simple decorator which allows arguments for the exceptions to catch and also
for the exception to later reraise from the caught exception
- this simplifies the code within the version manager when we deal with versions that could be integer strings
@nosahama nosahama requested review from a team as code owners May 29, 2024 13:09
@nosahama nosahama force-pushed the nosahama/resolve-minus-1-as-latest-version branch 3 times, most recently from d961ed1 to 59046e7 Compare May 29, 2024 13:46
@nosahama nosahama changed the title refactor: consolidate version resolution and validation logic refactor: consolidate schema version resolution and validation logic May 29, 2024
@nosahama nosahama force-pushed the nosahama/resolve-minus-1-as-latest-version branch from 59046e7 to f83cd9d Compare May 29, 2024 13:51
- this will replace the previously added `SchemaVersionManager`
- it will handle the resolution and validation of versions
- easy work between numeric and string version tags, keeping only the numeric version internally
…oller

- update the signature of `subject_version_get`
- update the signature of `subject_version_delete_local`
- update the signature of `subject_version_referencedby_get`
- change the various references/dependencies to build the version class
- replace `ResolvedVersion` references with builtin `int` to mimic the initial intention and encapsulate the `str`/`int` version logic to the `Version` class.
@nosahama nosahama force-pushed the nosahama/resolve-minus-1-as-latest-version branch 2 times, most recently from 19b02ee to 43d7ed0 Compare June 3, 2024 16:51
@nosahama nosahama force-pushed the nosahama/resolve-minus-1-as-latest-version branch from 43d7ed0 to cf1facb Compare June 3, 2024 19:56
@nosahama nosahama force-pushed the nosahama/resolve-minus-1-as-latest-version branch from 5f66a4a to 9c02e37 Compare June 4, 2024 16:38
karapace/schema_references.py Show resolved Hide resolved
karapace/schema_registry_apis.py Outdated Show resolved Hide resolved
karapace/in_memory_database.py Show resolved Hide resolved
- we create the `Version` objects in the API controller
using the `Versioner.V()` factory method, but we've moved this statement inline as an argument to the dependent functions.
This prevents us from overriding the `version` path parameter.
@eliax1996 eliax1996 merged commit a189931 into main Jun 5, 2024
8 checks passed
@eliax1996 eliax1996 deleted the nosahama/resolve-minus-1-as-latest-version branch June 5, 2024 15:47
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.

3 participants