nixos module rework #212
nixos module rework #212
Conversation
addresses a few complaints that were raised about the state of the nixos module component by rewriting it to utilize json dumping in python and json loading in nix instead of converting python objects straight to nix code.
| self.map(basename.replace(".nix", "_component.json")), | ||
| data=json.loads( | ||
| json.dumps( | ||
| self.source_component or self.parent, |
There was a problem hiding this comment.
I think this needs special casing for Address/NetLoc.
There was a problem hiding this comment.
A lot of components will require special handling, especially custom ones. I'm very much not sure how to handle this properly. The question is where do you draw the line honestly. I'd prefer as little special handling as possible.
There was a problem hiding this comment.
I'd prefer as little special handling as possible.
I don't disagree.
But it seems as if Address isn't evne JSON serializable, so currently the NixOSModule code would fail with such components, correct?
Ideally one could make classes JSON serializable in Python, but I'd have to do some more research, i.e. not now.
src/batou_ext/nixos.py
Outdated
| sensitive_data=True, | ||
| ); | ||
| in { | ||
| imports = [(module (args // { inherit component; }))]; |
There was a problem hiding this comment.
fwiw, I think what I'd prefer here is actually
{ component, ... }:
{ config, lib, pkgs, ... }:
{
}i.e. a function that generates a module given the component context instead of mixing this together with _module.args (if this ever gets a component-key, it will be funny).
I'm not sure if there's a transition path that's not just a pile of hacks, so we probably have no choice and need to leave this.
There was a problem hiding this comment.
Honestly, that is a better version.
Transitioning would have to be manual but could just be part of any regular maintenance. The change is not too drastic that we'd have to account for it in this component.
tiny change but I'd change it to just
component:
{ config, lib, pkgs, ... }:
{
}
since there are no other arguments to pass here
There was a problem hiding this comment.
since there are no other arguments to pass here
fine by me.
This will require an entry in the release notes then, though.
There was a problem hiding this comment.
100%, this is a breaking change
This PR addresses a few complaints that were raised about the state of the NixOS module component by rewriting it to utilize dumping relevant context as json in python and sourcing that json in nix instead of attempting to convert python objects straight to nix code.
TODOs