Skip to content

Refactor core API based on Pydantic models and direct loading from external sources #18

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

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

swrichards
Copy link
Contributor

@swrichards swrichards commented Nov 14, 2024

Based on discussions related both to PodiumD integration and broader lessons learned with regards to configuration management, this PR implements the first (big) iteration on the new design: step configs are loaded directly from external sources and are fully typed and validated using Pydantic.

This PR has removed two hooks from the API (the configuration check, and the self test), not because they are not important, but because such "liveness" and "health" checks arguably belong elsewhere. Documentation generation and multi-file yaml sources will be implemented in an upcoming iteration.

@swrichards swrichards force-pushed the swr/pydantic-based-setup branch 2 times, most recently from aceca0d to d594a46 Compare November 14, 2024 16:31
@swrichards swrichards requested a review from stevenbal November 14, 2024 16:31
@swrichards
Copy link
Contributor Author

@stevenbal This is not quite done, I need to do another pass for docstrings, some naming issues, and so on, but if you have a moment this week, it would be great to get a high level sign-off on the basic approach here before I publish it.

Copy link
Contributor

@stevenbal stevenbal left a comment

Choose a reason for hiding this comment

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

Looks good overall! Some minor remarks/questions

@swrichards swrichards force-pushed the swr/pydantic-based-setup branch 16 times, most recently from 11d2dc2 to 46eccfa Compare November 21, 2024 07:27
@swrichards swrichards marked this pull request as ready for review November 21, 2024 08:19
@swrichards swrichards force-pushed the swr/pydantic-based-setup branch 4 times, most recently from 2da8c93 to c7e38cb Compare November 21, 2024 14:36
@stevenbal stevenbal self-requested a review November 21, 2024 14:46
@stevenbal
Copy link
Contributor

Looks good to me! Like you mentioned, the naming issues can be tackled in a separate PR

@swrichards swrichards force-pushed the swr/pydantic-based-setup branch 5 times, most recently from 6f219b8 to c894d95 Compare November 21, 2024 18:24
@swrichards swrichards force-pushed the swr/pydantic-based-setup branch from fb9bf86 to eed63d1 Compare November 25, 2024 14:31
@swrichards swrichards merged commit a9dad5f into main Nov 25, 2024
17 checks passed
@stevenbal stevenbal deleted the swr/pydantic-based-setup branch January 23, 2025 08:09
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