Skip to content

Conversation

giograno
Copy link
Member

@giograno giograno commented Aug 30, 2025

Motivation

This PR improves the type hints for the TaggingService constructor by making explicit the nullability of the arguments.

py-avro-schema would raise an error with the existing implementation since the arguments were not nullable (i.e., not str | None) and None would not be a good default for a string.

In cases like this, mypy would raise an error: error: Incompatible default for argument "val" (default has type "None", argument has type "str") [assignment]).

@giograno giograno added this to the Playground milestone Aug 30, 2025
@giograno giograno requested a review from thrau August 30, 2025 15:47
@giograno giograno self-assigned this Aug 30, 2025
@giograno giograno added semver: patch Non-breaking changes which can be included in patch releases skip-docs Pull request does not require documentation changes labels Aug 30, 2025
Copy link

github-actions bot commented Aug 30, 2025

Test Results - Preflight, Unit

22 117 tests   20 379 ✅  7m 26s ⏱️
     1 suites   1 738 💤
     1 files         0 ❌

Results for commit 14c4fd6.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 30, 2025

Test Results (amd64) - Acceptance

7 tests   5 ✅  3m 9s ⏱️
1 suites  2 💤
1 files    0 ❌

Results for commit 14c4fd6.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 30, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 20m 39s ⏱️
5 003 tests 4 402 ✅ 601 💤 0 ❌
5 009 runs  4 402 ✅ 607 💤 0 ❌

Results for commit 14c4fd6.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 30, 2025

LocalStack Community integration with Pro

    2 files      2 suites   1h 43m 56s ⏱️
4 632 tests 4 189 ✅ 443 💤 0 ❌
4 634 runs  4 189 ✅ 445 💤 0 ❌

Results for commit 14c4fd6.

♻️ This comment has been updated with latest results.

@giograno giograno force-pushed the tagging-service-typing branch from cd975e5 to 4b35eab Compare September 1, 2025 09:27
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

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

LGTM! great for a quick fix for the schema creation. as i mentioned in the channel, for a long-term solution we may have to think about this differently, but this will unblock our schema creation test at least!

@giograno giograno merged commit 3ac3813 into main Sep 1, 2025
40 checks passed
@giograno giograno deleted the tagging-service-typing branch September 1, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases skip-docs Pull request does not require documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants