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

WIP: Use pydantic for config panels and app install #1603

Closed
wants to merge 7 commits into from

Conversation

Axolotle
Copy link
Contributor

TLDR:
Use pydantic for config panel and app install form parsing/validation.
PR is not yet ready for review, but i would like to know if it's worth continuing or not ;)
I guess it's a bit too much changes...

While working on the POC migration to FastAPI/typer I started working with pydantic (the parser/validator behind some features of FastAPI) and found it pretty cool. I thought we could give it a try for handling config panels and other dynamic forms like app install and maybe more.

  • It is based on model declaration (which can be dynamicly created)
  • It perform most of the basic parsing/validation directly from python type hints
  • Allow custom types
  • Combine parsing and validation in quite the same fashion
  • Use type coercion rather than strictness and errors (for example, on a model's field typed as a basic python bool, it can receive a string like "yes" and still manage to auto parse it as True) which is useful for our python/bash context.
  • Can export models declaration into json schema for IDEs, swagger or custom implementations.
  • It has a special class for settings management that could be used at some point?

So here's a WIP trying to integrate it to this complex datastructure which features a toml syntax to build custom multi-level and conditional forms that integrate inputs of many types, display/readonly components and has to be displayed as a CLI input sequence and thru API web forms.

The proposed implementation is roughly:

  • parse/validate config.toml declarations with pydantic models to instantiate a config: ConfigPanel
  • based on this config, build a dynamic pydantic model that will act as settings: Form hydrated with current settings.yml
  • depending on the interface:
    • CLI: use config to iterate over what to ask/display and parse/validate user inputs thru settings assignements.
    • API: directly try to assign new values to settings while still checking conditions from visible|enabled props.
  • finally, depending on the config panel type:
    • App: export settings as an env (only strings) and pass it to bash magic helpers.
    • Setting+Domain: export none-default settings as yaml with "real" types, use touched settings (only the ones that changed) and update what's needed.

The code is split into two files:

  • form.py: Custom type to handle bash list, Options models for every implemented display/inputs components, basic form factory, CLI displayer/prompter, API filler, evaluation functions. This can be used directly by app_install() for install form.
  • configpanel.py: which is more or less the config panel implementation built over base stuff in form.py. (ConfigPanel containing Panels containing Sections containing Options) and other specific stuff. (Haven't removed the actual implementation to be able to compare to it easily)

Small new thing i have in mind:

  • Types select and tags can receive an item_type = "string|color|number|date|time|email|path|url" default to string that will parse/validate every items in the list with the correct type, so you can ask for a list of emails and be sure all are valid emails without reinventing regex to match already defined types. (could be extended to allow file|app|domain|user|group)
  • Managed to build a first version of a json schema for config panels but i can't paste it here (too long)

PR STATUS

Not yet ready for review i guess, need to update tests and test it moar + the todo.
This does not yet brings that much new stuff but open the door for more type safe code, i think pydantic could be used in other part of the core code and we could rely on it for most of the validation/parsing we need and use it to define our datastructures. I tried to test it on config panel because i thought it was the most complex but idk.
Also if we definitly plan to move to FastAPI, i guess it will be an easier migration.

TODO

  • export a json schema for app's config panels and app's manifest.
  • iron things out if you think pydantic would be a nice addition to core code:
    • i18n of errors
    • Accept None for bool (not set)?
    • Redact stuff on OperationLogger
    • file handling which is not really implemented yet (handle API case + temp file for context evaluation)
    • required options are not very clear to me in case on config panel, does config panel required options have to be set at install?
      • If not Config.get() will yell at settings validation since it may not be defined.
      • If so, what about required options in conditional section (not visible), action section (required only for action, not as setting)
      • Need to dig a bit to see what could be possible, for now the settings in Config.set() are not validated at instantiation to allow the prompt/fill a chance to define a value, but this is not tottaly satisfying since it mess up a bit with "detecting what changed" since originally the form is unparsed/unvalidated.
    • reflect some changes to webadmin

Possible improvments

  • Should pretty easy to separate "" from None and allow multiple values in CLI with a custom type: "null|nil|none|_none")?
  • could use part of this code to test apps in package_check?
  • Could add some more types that are available in pydantic and some extra properties to current types like min_length|max_length of constr
  • try to implement an AppManifest pydantic model integrating the app install dynamic form.
  • API: validate the whole form and return multiple errors if any (not only the first encountered as of today)
  • API: app_config_get(full=True) could return a JSON Schema directly instead of our custom toml specs in the web admin (need to update the current web admin config panel/custom forms interpreter to this more generic JSON schema approach)
  • Find a way to use directly an adapted settings model when CLI prompting/ API filling instead of relying on config (repr of config toml stuff) with nested stuff and extra information on each field.
    • This would open the door for defining SettingConfigPanel and DomainConfigPanel as pydantic models directly and get rid of their .toml.

How to test

# install pydantic
pip install pydantic

Also install python-email-validator to use pydantic EmailStr type. This is extra dep so maybe we can use a simpler custom regex validator but this lib handle international stuff (but i'm not sure we do).

pip install pydantic[email]

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@zamentur
Copy link
Member

Note: i use also configpanel into backup pr https://github.com/YunoHost/yunohost/pull/531/files

@Axolotle
Copy link
Contributor Author

Note: i use also configpanel into backup pr https://github.com/YunoHost/yunohost/pull/531/files

oh god

@Axolotle
Copy link
Contributor Author

superseeded by #1677

@Axolotle Axolotle closed this Oct 30, 2023
@alexAubin alexAubin deleted the pydantic-configpanel branch October 30, 2023 13:17
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