Skip to content

Draft: Use pydantic to represent configs. Rework constructors

Cal Pratt requested to merge cpratt34/yaml-loader into master

Description

The way configurations are represented today makes the loading of the server hard to reason about and to debug. The setup of the yaml constructors and json schemas allows room for errors, as there is no way to validate that the json schema accurately reflects interface of the yaml constructor. Additionally, yaml.add_constructor does not allow us to do things like use context managers for the creation of external resource components, as there is no easy way to inject context into the loading logic.

This change tries to achieve two things:

  1. Replacing the schemas with pydantic models.

    This will allow us to achieve greater type safety when converting from the yaml representation into the actual objects. I started off this refactoring by first taking each schema and converting it, slowly adding all of the missing references to configs which I found along the way.

  2. Separate the actual class construction from the yaml loading.

    Given a pydantic model representing the configs we can create the server instances in a much more controlled manner. To ensure we do not create duplicates of objects, a context-var based wrapper has been applied to the individual constructor methods. This pattern should allow us to more easily add new configs, and opens up the door to add context to the constructor calls. e.g. an ExitStack for context manager based resources.

Notes: ruamel.yaml provides a better alternative to PyYAML. It allows setting the constructors on a yaml loader instance rather than the module itself, meaning constructors do not bleed into the global state, and will not interfere with other loaders in multi-threaded contexts.

TODO:

  • Add missing validators on some of the config classes
  • Perform a rundown through the old constructors to verify parity
  • Add additional tests which load full configs (preferably based on configs used in production scenarios)
  • Add targeted unit tests the constructor_cache wrapper
Edited by Cal Pratt

Merge request reports