generate Python data models from module options #285

Merged
fricklerhandwerk merged 31 commits from fricklerhandwerk/Fediversity:generate-module-options into main 2025-05-01 01:26:54 +02:00

edit by kiara: see #275.

this shows a proof of concept for generating Django forms from NixOS modules

it still needs lots of cleanup (and probably unbreaking the deployment action), and a sane contributor workflow when updating the underlying NixOS modules.

edit by kiara: see #275. this shows a proof of concept for generating Django forms from NixOS modules it still needs lots of cleanup (and probably unbreaking the deployment action), and a sane contributor workflow when updating the underlying NixOS modules.
kiara reviewed 2025-03-31 16:33:49 +02:00
@ -18,3 +32,4 @@
inputsFrom = [ (pkgs.callPackage ./nix/package.nix { }) ];
packages = [
pkgs.npins
pkgs.jq
Owner

where is this used btw? maybe clarify in a comment there

where is this used btw? maybe clarify in a comment there
fricklerhandwerk marked this conversation as resolved
kiara reviewed 2025-03-31 16:34:48 +02:00
@ -15,0 +22,4 @@
{
}
''
${codegen} --input ${schema} | sed '/from pydantic/a\
Owner

what's the replacement for? maybe clarify in a comment

what's the replacement for? maybe clarify in a comment
fricklerhandwerk marked this conversation as resolved
kiara reviewed 2025-03-31 16:36:01 +02:00
@ -1,5 +1,16 @@
{
"pins": {
"clan-core": {
Owner

iiuc this is still unused (as per your comment below)?

iiuc this is still unused (as per your comment below)?
fricklerhandwerk marked this conversation as resolved
kiara reviewed 2025-03-31 16:38:45 +02:00
@ -0,0 +1,430 @@
# TODO(@fricklerhandwerk):
# temporarily copied from
# https://git.clan.lol/clan/clan-core/src/branch/main/lib/jsonschema/default.nix
# to work around a bug in JSON Schema generation; this needs a PR to upstream
Owner

looking forward to the PR, both to clarify/explain the diff, as well as to separate concerns - as far as i'm concerned (until merged) feel free to pull this from such a fork rather than inlining here

looking forward to the PR, both to clarify/explain the diff, as well as to separate concerns - as far as i'm concerned (until merged) feel free to pull this from such a fork rather than inlining here
fricklerhandwerk marked this conversation as resolved
kiara reviewed 2025-03-31 17:59:34 +02:00
kiara left a comment
Owner

thanks! i added a few comments, hoping to better understand things still

thanks! i added a few comments, hoping to better understand things still
@ -0,0 +1,64 @@
/**
Deployment options as to be presented in the front end.
These are converted to JSON schema in order to generate front-end forms etc.
Owner

with this form generation, we may need to still further consider visual choices down the road, such as (in the current form) input type="password"

with this form generation, we may need to still further consider visual choices down the road, such as (in the current form) `input type="password"`
@ -0,0 +10,4 @@
}:
let
inherit (lib) types mkOption;
in
Owner

down the road we may want to look into guaranteeing our different components use and expect the same structure, but having any check here definitely improves on what we had already :)

down the road we may want to look into guaranteeing our different components use and expect the same structure, but having any check here definitely improves on what we had already :)
fricklerhandwerk marked this conversation as resolved
panel/.gitignore Outdated
@ -1,6 +1,7 @@
# Nix
.direnv
result*
src/panel/configuration/schema.py
Owner

for review purposes, it'd be nice to demonstrate sample contents of such a generated file in the PR description

for review purposes, it'd be nice to demonstrate sample contents of such a generated file in the PR description
@ -0,0 +20,4 @@
# remove _module attribute from options
clean = opts: builtins.removeAttrs opts [ "_module" ];
# throw error if option type is not supported
Owner

if we were to at some point plug existing nix modules into this, we might consider a schema export ignoring (rather than crashing on) fields too complex in their defaults/type. i don't know if clan cares for a PR in that direction, otherwise we could fork.

if we were to at some point plug existing nix modules into this, we might consider a schema export ignoring (rather than crashing on) fields too complex in their defaults/type. i don't know if clan cares for a PR in that direction, otherwise we could fork.
fricklerhandwerk marked this conversation as resolved
@ -0,0 +41,4 @@
{
string = ''"${toString value}"'';
integer = toString value;
boolean = if value then "true" else "false";
Owner

not just toString as well?

not just `toString` as well?
fricklerhandwerk marked this conversation as resolved
@ -0,0 +75,4 @@
in
{
options.services.${name} = {
${options}
Owner

to what extent would this differ from deployment/options.nix?

to what extent would this differ from `deployment/options.nix`?
fricklerhandwerk marked this conversation as resolved
@ -0,0 +2,4 @@
import textwrap
# TODO(@fricklerhandwerk): mix this in as a method on the Pydantic Schema itself
def generate_module(schema, service_name):
Owner

could you add a docstring (on module or function) on what this file does?

could you add a docstring (on module or function) on what this file does?
fricklerhandwerk marked this conversation as resolved
@ -47,3 +45,3 @@
self.assertTrue(config.parsed_value.mastodon.enable)
# this should not have changed
self.assertFalse(config.parsed_value.peertube.enable)
self.assertIsNone(config.parsed_value.peertube)
Owner

to better understand how this works, would you know how it decided not to send this sub-object?

to better understand how this works, would you know how it decided not to send this sub-object?
Author
Owner

here it's simply not setting those fields because they're optional

here it's simply not setting those fields because they're optional
fricklerhandwerk marked this conversation as resolved
@ -0,0 +40,4 @@
}
""")
actual = to_module.generate_module(Example.schema(), "myservice")
Owner

were we not converting from nix (thru json schema) to django, rather than from django to nix? i'm confused on our moving parts here so could maybe do with a few extra comments and/or pr description

were we not converting from nix (thru json schema) to django, rather than from django to nix? i'm confused on our moving parts here so could maybe do with a few extra comments and/or pr description
Author
Owner

Yes, this is an artefact and I'll remove it.

Yes, this is an artefact and I'll remove it.
kiara marked this conversation as resolved
@ -95,1 +70,4 @@
# TODO(@fricklerhandwerk):
# this is broken after changing the form view.
# but if there's no test for it, how do we know it ever worked in the first place?
Owner
@kevin @lois
fricklerhandwerk marked this conversation as resolved
@ -114,3 +97,1 @@
"pixelfed": deployment_params['pixelfed']['enable'],
"mastodon": deployment_params['mastodon']['enable']
}
"services": deployment_params.json(),
Owner

🎉

🎉
fricklerhandwerk marked this conversation as resolved
Author
Owner

Oh yeah the first two commits also contain sketches to create modules from Python, but that was code from before I settled on the other direction (and also the second one is not particularly smart, because we could just build an expression directly rather than a string). As said, this needs cleanup and testing.

Oh yeah the first two commits also contain sketches to create modules from Python, but that was code from before I settled on the other direction (and also the second one is not particularly smart, because we could just build an expression directly rather than a string). As said, this needs cleanup and testing.
Owner

fwiw i tried a rebase.
the one annoying conflict i found pertained to reconciling passing arguments to the view, which in this case i resolved by having the view reuse the available info for now.

fwiw i tried a [rebase](https://git.fediversity.eu/fricklerhandwerk/Fediversity/compare/generate-module-options...kiara/Fediversity:generate-module-options-rebase). the one annoying conflict i found pertained to reconciling passing arguments to the view, which in this case i resolved by having the view reuse the available info for now.
kiara reviewed 2025-04-08 12:45:53 +02:00
@ -0,0 +182,4 @@
description = lib.optionalAttrs (option ? description) {
description = option.description.text or option.description;
};
exposedModuleInfo = lib.optionalAttrs true (makeModuleInfo {
Owner

i see you moved this - why was that, and while we're at it why would anyone ever us a no-op like optionalAttrs true in the first place? symmetric considerations?

i see you moved this - why was that, and while we're at it why would anyone ever us a no-op like `optionalAttrs true` in the first place? symmetric considerations?
fricklerhandwerk marked this conversation as resolved
kiara reviewed 2025-04-08 12:46:31 +02:00
@ -0,0 +127,4 @@
# parse options to jsonschema properties
properties = lib.mapAttrs (_name: option: (parseOption' (path ++ [ _name ]) option)) options';
# TODO: figure out how to handle if prop.anyOf is used
isRequired = prop: !(prop ? default || prop."$exportedModuleInfo".required or false);
Owner

what was this change about?

what was this change about?
Author
Owner

this was required to somehow propagate that fixed attrs are required fields

this was required to somehow propagate that fixed attrs are required fields
fricklerhandwerk marked this conversation as resolved
kiara reviewed 2025-04-08 12:47:12 +02:00
@ -0,0 +381,4 @@
# return jsonschema property definition for attrs
then
default
// (lib.reqursiveUpdate exposedModuleInfo {
Owner

what was the change here about?

what was the change here about?
fricklerhandwerk marked this conversation as resolved
fricklerhandwerk force-pushed generate-module-options from a6e5cbb67e to 9c73741559 2025-04-16 13:42:37 +02:00 Compare
fricklerhandwerk added 5 commits 2025-04-16 17:53:29 +02:00
problem: attrs fields are now required (which is correct!)
fix types and defaults
Some checks failed
/ check-pre-commit (pull_request) Successful in 20s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Failing after 4m21s
851f9b13cb
fricklerhandwerk added 1 commit 2025-04-16 18:01:22 +02:00
Merge branch 'main' into generate-module-options
Some checks failed
/ check-pre-commit (pull_request) Successful in 20s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Failing after 17m30s
69eaa289e1
fricklerhandwerk added 3 commits 2025-04-16 18:34:22 +02:00
fix data types for displaying defaults correctly
Some checks failed
/ check-pre-commit (pull_request) Successful in 20s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Failing after 17m32s
bbab1a3e3f
fricklerhandwerk added 1 commit 2025-04-22 14:38:03 +02:00
Merge branch 'main' into generate-module-options
Some checks failed
/ check-pre-commit (pull_request) Successful in 20s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Failing after 17m30s
70be285334
fricklerhandwerk added 6 commits 2025-04-22 16:42:23 +02:00
revert unneeded changes
All checks were successful
/ check-pre-commit (pull_request) Successful in 20s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Successful in 1m37s
0bae4c4a27
fricklerhandwerk added 1 commit 2025-04-24 11:15:54 +02:00
move to inner
All checks were successful
/ check-pre-commit (pull_request) Successful in 20s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Successful in 1m24s
09239e6245
fricklerhandwerk changed title from WIP: generate Python data models from module options to generate Python data models from module options 2025-04-30 15:22:30 +02:00
fricklerhandwerk added 1 commit 2025-04-30 15:22:39 +02:00
Merge branch 'main' into generate-module-options
All checks were successful
/ check-pre-commit (pull_request) Successful in 11m53s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Successful in 1m20s
/ check-deployment-basic (pull_request) Successful in 11m47s
cc8cd52ec6
Author
Owner

@kiara is this small enough to be reviewable for you? I'd maybe split adding the new Python package into a separate commit and squash the rest. Making it nicer (specifically: simplifying the baroque module declaration to be like one would expect a module to be) is blocked on the JSON schema converter being clumsy with types, and it doesn't seem worth the effort to fix that at the moment.

@kiara is this small enough to be reviewable for you? I'd maybe split adding the new Python package into a separate commit and squash the rest. Making it nicer (specifically: simplifying the baroque module declaration to be like one would expect a module to be) is blocked on the JSON schema converter being clumsy with types, and it doesn't seem worth the effort to fix that at the moment.
kiara reviewed 2025-04-30 15:54:49 +02:00
@ -0,0 +4,4 @@
These are converted to JSON schema in order to generate front-end forms etc.
For this to work, options must not have types `functionTo` or `package`, and must not access `config` for their default values.
The options are written in a cumbersome way because the JSON schema converter can't evaluate a submoule option's default value, which thus all must be set to `null`.
Owner

typo

typo
fricklerhandwerk marked this conversation as resolved
kiara reviewed 2025-04-30 15:57:43 +02:00
@ -0,0 +5,4 @@
For this to work, options must not have types `functionTo` or `package`, and must not access `config` for their default values.
The options are written in a cumbersome way because the JSON schema converter can't evaluate a submoule option's default value, which thus all must be set to `null`.
*/
Owner

interesting challenge, i wonder if we may need some better solution to this down the line

interesting challenge, i wonder if we may need some better solution to this down the line
Author
Owner

Yes, essentially the JSON schema converter needs to be aware of $defs. But that feels like half a rewrite (which I concluded after trying to weasel around that effort, unsuccessfully).

Yes, essentially the JSON schema converter needs to be aware of [`$defs`](https://json-schema.org/understanding-json-schema/structuring#defs). But that feels like half a rewrite (which I concluded after trying to weasel around that effort, unsuccessfully).
Owner

thanks, added at #195 as a note on where we stand on this

thanks, added at #195 as a note on where we stand on this
kiara marked this conversation as resolved
kiara reviewed 2025-04-30 16:05:17 +02:00
@ -95,1 +67,4 @@
# TODO(@fricklerhandwerk):
# this is broken after changing the form view.
# but if there's no test for it, how do we know it ever worked in the first place?
Owner

maybe file in the issue tracker if this is a regression induced prior to this PR, as it would then be unrelated to it

maybe file in the issue tracker if this is a regression induced prior to this PR, as it would then be unrelated to it
Author
Owner

This is tracked as #276

This is tracked as https://git.fediversity.eu/Fediversity/Fediversity/issues/276
kiara marked this conversation as resolved
kiara reviewed 2025-04-30 16:10:24 +02:00
@ -121,2 +93,2 @@
},
},
"deployment_status": deployment_status,
"services": deployment_params.json(),
Owner

did this change not also require fricklerhandwerk/Fediversity#1/files (as alluded to at #285 (comment))?

did this change not also require https://git.fediversity.eu/fricklerhandwerk/Fediversity/pulls/1/files#diff-9c2dbac7d46dc88eb45edcf89c466784778361bf (as alluded to at https://git.fediversity.eu/Fediversity/Fediversity/pulls/285#issuecomment-5607)?
Author
Owner

Hm no, I think this is alright

Hm no, I think this is alright
Owner

how does that even work? the template still wants to show .url there which you removed while we don't otherwise set anything like it?
i worry you may have overlooked this as earlier you mentioned relying wholly on CI to verify intended behavior.

how does that even work? the template still wants to show `.url` there which you [removed](https://git.fediversity.eu/Fediversity/Fediversity/pulls/285/files#diff-de06c10854bd6e90e4d3acc5f00f0c1fdbb89a00) while we don't otherwise set anything like it? i worry you may have overlooked this as earlier you mentioned relying wholly on CI to verify intended behavior.
Author
Owner

Maybe. As noted, this interaction needs a test.

Maybe. As noted, this interaction needs a test.
Owner

i've filed #326 to track this.

i've filed #326 to track this.
kiara reviewed 2025-04-30 16:27:06 +02:00
@ -33,0 +43,4 @@
codegen = "${python3Packages.datamodel-code-generator}/bin/datamodel-codegen";
pydantic = runCommand "schema.py" { } ''
${codegen} --input ${schema} | sed '/from pydantic/a\
from drf_pydantic import BaseModel' > $out
Owner

this sed expression looks weird, not ending in a slash, having the a\<BREAKLINE>, how does that work?

this sed expression looks weird, not ending in a slash, having the `a\<BREAKLINE>`, how does that work?
Author
Owner

sed magic... it replaces the matched line with the following line. 🤷

sed magic... it replaces the matched line with the following line. 🤷
kiara marked this conversation as resolved
kiara approved these changes 2025-04-30 16:27:11 +02:00
fricklerhandwerk added 2 commits 2025-05-01 01:11:13 +02:00
add explanatory comment
All checks were successful
/ check-pre-commit (pull_request) Successful in 12m12s
/ check-peertube (pull_request) Successful in 18s
/ check-panel (pull_request) Successful in 1m26s
/ check-deployment-basic (pull_request) Successful in 12m6s
b16a60603a
Author
Owner

TODO: commit message on squash

this shows a proof of concept for generating Django forms from NixOS modules

note that the form behavior is still rather clumsy and doesn't exactly map to the module semantics:
- since forms can only be sent wholesale, empty form fields will show up as empty strings
  and break validation without additional cleanup (not done here)
- it's not possible to faithfully translate `type = submodule { /* ... */}; default = {};`, since the default
  is translated to an empty dict `{}`. this is because the JSON schema converter does not preserve type information.
  this can be added by making it use `$defs` [1], but that would likely amount to half a rewrite
- there's a glitch in enum default values that needs to be fixed in `datamodel-code-generator` [0]


[0]: https://github.com/koxudaxi/datamodel-code-generator/blob/dd44480359c81257e55be18afefec0b3f0267ccd/src/datamodel_code_generator/parser/base.py#L1015
[1]: https://json-schema.org/understanding-json-schema/structuring#defs


a generated file will be placed into the source (by the development shell and the package respectively)
that declares Pydantic types from which to render the form. it looks something like this:

```python
from __future__ import annotations

from enum import Enum
from typing import Optional

from pydantic import BaseModel, Extra, Field
from drf_pydantic import BaseModel


class Domain(Enum):
    fediversity_net = 'fediversity.net'

# ...

class Model(BaseModel):
    class Config:
        extra = Extra.forbid

    domain: Optional[Domain] = Field(
        'fediversity.net',
        description='Apex domain under which the services will be deployed.\n',
    )

  # ...
```
TODO: commit message on squash ```` this shows a proof of concept for generating Django forms from NixOS modules note that the form behavior is still rather clumsy and doesn't exactly map to the module semantics: - since forms can only be sent wholesale, empty form fields will show up as empty strings and break validation without additional cleanup (not done here) - it's not possible to faithfully translate `type = submodule { /* ... */}; default = {};`, since the default is translated to an empty dict `{}`. this is because the JSON schema converter does not preserve type information. this can be added by making it use `$defs` [1], but that would likely amount to half a rewrite - there's a glitch in enum default values that needs to be fixed in `datamodel-code-generator` [0] [0]: https://github.com/koxudaxi/datamodel-code-generator/blob/dd44480359c81257e55be18afefec0b3f0267ccd/src/datamodel_code_generator/parser/base.py#L1015 [1]: https://json-schema.org/understanding-json-schema/structuring#defs a generated file will be placed into the source (by the development shell and the package respectively) that declares Pydantic types from which to render the form. it looks something like this: ```python from __future__ import annotations from enum import Enum from typing import Optional from pydantic import BaseModel, Extra, Field from drf_pydantic import BaseModel class Domain(Enum): fediversity_net = 'fediversity.net' # ... class Model(BaseModel): class Config: extra = Extra.forbid domain: Optional[Domain] = Field( 'fediversity.net', description='Apex domain under which the services will be deployed.\n', ) # ... ``` ````
fricklerhandwerk merged commit 6100b278b6 into main 2025-05-01 01:26:54 +02:00
fricklerhandwerk deleted branch generate-module-options 2025-05-01 01:26:55 +02:00
Sign in to join this conversation.
No description provided.