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

Yet another attempt to remove APIv3 magic #3355

Merged
merged 5 commits into from
Aug 9, 2024

Conversation

FrostyX
Copy link
Member

@FrostyX FrostyX commented Aug 3, 2024

An attempt number 356 to remove magic and excess complexity from our APIv3 schemas. I won't sleep soundly until we figure it out, so in order to preserve my own sanity, here we go.

  • I am keeping schemas as @dataclass, we already figured out that it is what we want
  • I am keeping shared fields in fields.py but moving only once used fields definitions directly to schemas
  • I am removing some magic:
    • It is no longer possible to mark all fields as required. The largest schema that did it had 3 fields.
    • We are no longer magically matching schema fields by their name with their definitions in fields.py
    • Because of this ^^, we don't need the hack for renaming field names
  • I simplified a lot of code in Schema, InputSchema, and ParamsSchema

When reviewing, I recommend going commit by commit.

In a separate follow-up PR, I'd like to take a look at the schema class hierarchy.

@FrostyX FrostyX force-pushed the api-schema-cleanup-2 branch 2 times, most recently from 2d323d9 to 8d5c46c Compare August 3, 2024 11:03
Copy link
Member

@nikromen nikromen left a comment

Choose a reason for hiding this comment

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

+1, very nice improvement and keeping the dataclasses will help us with fedora-copr/debate#9 (if we will ever get there :D)

@FrostyX FrostyX merged commit ed6d972 into fedora-copr:main Aug 9, 2024
7 checks passed
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.

2 participants