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

[FEATURE] Refactor serializers to pydantic #459

Open
uptickmetachu opened this issue Feb 13, 2024 · 3 comments
Open

[FEATURE] Refactor serializers to pydantic #459

uptickmetachu opened this issue Feb 13, 2024 · 3 comments
Assignees
Labels
feature-request New feature or request

Comments

@uptickmetachu
Copy link

uptickmetachu commented Feb 13, 2024

What are your thouhgts on refactoring the serializers to pydantic instead of marshmallow? I would be happy to make a PR if you thought it woudl be useful.

https://docs.pydantic.dev/latest/

I can see several benefits:

  1. Better type hints throughout the code base. The code currently uses the following settings access patern.
config.active.integrations.get("atlassian")
.get("confluence")
.get("space"),

It would be advantageous to get the proper types instead of relying on default getters. This would simplify refactoring and provide extra safety.

  1. Better configuration parsing.
    Currently configuration must come from config.yaml and environment variables. For improved flexibility and testing ergonomics it would be nice to to set configuration from a config.yaml with environment variable overrides. Pydantic's settings management is both powerful and flexible.

  2. Generating pydantic models from 3rd party (confluence/jira/opsgenie/pagerduty) apis will also improve DX + type safety.

Love to hear your thoughts :).

@uptickmetachu uptickmetachu added the feature-request New feature or request label Feb 13, 2024
@uptickmetachu uptickmetachu changed the title [FEATURE] Refactor types to pydantic [FEATURE] Refactor serializers to pydantic Feb 13, 2024
@echoboomer
Copy link
Owner

@uptickmetachu Thanks for the contribution. I'll check this out as soon as I can.

@echoboomer
Copy link
Owner

@uptickmetachu sorry it took me a bit to get back to you - honestly, after doing some work on a project I just built, it might even be worth implementing FastAPI in-place of the bespoke Flask implementation which would also make it easier to implement Pydantic models. Any thoughts?

@uptickmetachu
Copy link
Author

uptickmetachu commented May 23, 2024

I'll be honest, I have written a mostly working version of incident bot using fast-api, sqlmodel etc with the goal of everything being type safe.

I haven't gotten around to using it as I haven't had time (as well!) to finish it off.

Would love to collaborate or share ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants