launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+427838@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lpcraft/config.py b/lpcraft/config.py
> index 328cd56..a472428 100644
> --- a/lpcraft/config.py
> +++ b/lpcraft/config.py
> @@ -30,7 +30,6 @@ class _Identifier(pydantic.ConstrainedStr):
>  class ModelConfigDefaults(
>      pydantic.BaseModel,
>      extra=pydantic.Extra.forbid,
> -    frozen=True,

I am not happy about that change, as immutable data structures have many advantages..., but it looks like this is necessary at this point, or at least it is the most straightforward way. Some other ways come to my mind, ...

Either create a complete new immutable data structure, or do not pass pydantic objects into _copy_output_paths and _copy_output_properties, then there would be no need to change the initial pydantic object.

>      alias_generator=lambda s: s.replace("_", "-"),
>      underscore_attrs_are_private=True,
>  ):
> @@ -49,7 +48,7 @@ class Output(ModelConfigDefaults):
>      paths: Optional[List[StrictStr]]
>      distribute: Optional[OutputDistributeEnum]
>      channels: Optional[List[StrictStr]]
> -    properties: Optional[Dict[StrictStr, StrictStr]]
> +    properties: Optional[Dict[StrictStr, Any]]

We also need to make at least dictionaries possible.

>      dynamic_properties: Optional[Path]
>      expires: Optional[timedelta]
>  


-- 
https://code.launchpad.net/~jugmac00/lpcraft/+git/lpcraft/+merge/427838
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/lpcraft:add-license-key-to-configuration into lpcraft:main.