Skip to content

Commit

Permalink
feat: return all schema validation errors
Browse files Browse the repository at this point in the history
  • Loading branch information
davide-armand committed Oct 3, 2024
1 parent a66dd00 commit d98176e
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 13 deletions.
6 changes: 4 additions & 2 deletions src/karapace/schema_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,14 @@ async def write_new_schema_local(
result = self.check_schema_compatibility(new_schema, subject)

if is_incompatible(result):
message = set(result.messages).pop() if result.messages else ""
LOG.warning(
"Incompatible schema: %s, incompatibilities: %s", result.compatibility, result.incompatibilities
)
compatibility_mode = self.get_compatibility_mode(subject=subject)
raise IncompatibleSchema(f"Incompatible schema, compatibility_mode={compatibility_mode.value} {message}")
raise IncompatibleSchema(
f"Incompatible schema, compatibility_mode={compatibility_mode.value}. "
f"Incompatibilities: {', '.join(result.messages)[:300]}"
)

# We didn't find an existing schema and the schema is compatible so go and create one
version = self.database.get_next_version(subject=subject)
Expand Down
2 changes: 1 addition & 1 deletion src/karapace/schema_registry_apis.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ async def compatibility_check(
result = SchemaCompatibility.check_compatibility(old_schema, new_schema, compatibility_mode)

if is_incompatible(result):
self.r({"is_compatible": False}, content_type)
self.r({"is_compatible": False, "incompatibilities": f"{', '.join(result.messages)[:300]}"}, content_type)
self.r({"is_compatible": True}, content_type)

async def schemas_list(self, content_type: str, *, request: HTTPRequest, user: User | None = None):
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/test_dependencies_compatibility_protobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ async def test_protobuf_schema_compatibility_dependencies(registry_async_client:
json={"schemaType": "PROTOBUF", "schema": evolved_schema, "references": evolved_references},
)
assert res.status_code == 200
assert res.json() == {"is_compatible": False}
assert not res.json().get("is_compatible")


@pytest.mark.parametrize("trail", ["", "/"])
Expand Down Expand Up @@ -271,7 +271,7 @@ async def test_protobuf_schema_compatibility_dependencies1(registry_async_client
json={"schemaType": "PROTOBUF", "schema": evolved_schema, "references": evolved_references},
)
assert res.status_code == 200
assert res.json() == {"is_compatible": False}
assert not res.json().get("is_compatible")


# Do compatibility check when message field is altered from referenced type to google type
Expand Down Expand Up @@ -339,7 +339,7 @@ async def test_protobuf_schema_compatibility_dependencies1g(registry_async_clien
json={"schemaType": "PROTOBUF", "schema": evolved_schema},
)
assert res.status_code == 200
assert res.json() == {"is_compatible": False}
assert not res.json().get("is_compatible")


# Do compatibility check when message field is altered from google type to referenced type
Expand Down Expand Up @@ -407,7 +407,7 @@ async def test_protobuf_schema_compatibility_dependencies1g_otherway(registry_as
json={"schemaType": "PROTOBUF", "schema": evolved_schema, "references": container_references},
)
assert res.status_code == 200
assert res.json() == {"is_compatible": False}
assert not res.json().get("is_compatible")


@pytest.mark.parametrize("trail", ["", "/"])
Expand Down Expand Up @@ -491,7 +491,7 @@ async def test_protobuf_schema_compatibility_dependencies2(registry_async_client
json={"schemaType": "PROTOBUF", "schema": evolved_schema, "references": evolved_references},
)
assert res.status_code == 200
assert res.json() == {"is_compatible": False}
assert not res.json().get("is_compatible")


SIMPLE_SCHEMA = """\
Expand Down
10 changes: 7 additions & 3 deletions tests/integration/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ async def test_compatibility_endpoint(registry_async_client: Client, trail: str)
json={"schema": json.dumps(schema)},
)
assert res.status_code == 200
assert res.json() == {"is_compatible": False}
assert not res.json().get("is_compatible")
assert res.json().get("incompatibilities") == "reader type: string not compatible with writer type: int"


@pytest.mark.parametrize("trail", ["", "/"])
Expand Down Expand Up @@ -537,7 +538,7 @@ def _test_cases():
json={"schema": json.dumps(schema)},
)
assert res.status_code == 200
assert res.json() == {"is_compatible": expected}
assert res.json().get("is_compatible") == expected


@pytest.mark.parametrize("trail", ["", "/"])
Expand Down Expand Up @@ -3254,7 +3255,10 @@ async def test_schema_non_compliant_name_in_existing(
assert res.status_code == 409
assert res.json() == {
"error_code": 409,
"message": "Incompatible schema, compatibility_mode=BACKWARD expected: compliant_name_test.test-schema",
"message": (
"Incompatible schema, compatibility_mode=BACKWARD. "
"Incompatibilities: expected: compliant_name_test.test-schema"
),
}

# Send compatibility configuration for subject that disabled backwards compatibility.
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/test_schema_protobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -1123,8 +1123,8 @@ async def test_protobuf_error(registry_async_client: Client) -> None:
expected=409,
expected_msg=(
# ACTUALLY THERE NO MESSAGE_DROP!!!
"Incompatible schema, compatibility_mode=BACKWARD "
"Incompatible modification Modification.MESSAGE_DROP found"
"Incompatible schema, compatibility_mode=BACKWARD. "
"Incompatibilities: Incompatible modification Modification.MESSAGE_DROP found"
),
)
print(f"Adding new schema, subject: '{testdata.subject}'\n{testdata.schema_str}")
Expand Down

0 comments on commit d98176e

Please sign in to comment.