feat: support for generating charmcraft YAML from Python classes by tonyandrewmeyer · Pull Request #1975 · canonical/operator

tonyandrewmeyer · 2025-08-11T23:05:31Z

A new package, ops-tools is added. This currently only provides a script to update charmcraft.yaml based on config and action Python classes, but is designed to also offer other tools in the future. It is released simultaneously with the other ops-* packages, but is not required by any of the others, or offered as an extra.

Charmers can use the exported functions (which take a Python class object and return a dictionary suitable for serialisation to YAML), but are expected to use the provided script (that works with a charmcraft.yaml file and Python modules).

Classes can provide a to_juju_schema() method if they need to provide YAML in a different way. The script will pass the generated config/action as a base to optionally work from.

The implementation is designed to work with the load_config and load_params functionality in ops. In particular, that means working with four types of class:

A generic Python class (no inheritance other than object).
Standard library dataclasses (I personally believe these are the best choice for almost all cases).
Pydantic dataclasses
Pydantic 2.x BaseModel classes

Only reference documentation is included in this PR (Preview). The intention is that there will be a follow-up PR that includes at least how-to documentation.

More details are available in the spec. This PR replaces #1702.

… don't catch the error themselves.

…break the config into multiple classes.

…if the secret isn't available, but avoids Juju calls when loading config.

…id secret IDs.

james-garner-canonical · 2025-08-29T02:06:10Z

+        if hasattr(field.default, 'default_factory')
+        else field.default_factory
+    )
+    # A hack to avoid importing Pydantic here.


What if ops-tools just depended on Pydantic? We don't want to add a heavy dependency to ops itself, but would this all be a lot easier if we included it in ops-tools?

I'm pretty strongly opposed to pulling Pydantic in, even for ops-tools. It would be a bit easier, but not a lot, I feel.

May I ask why -- is it the extra runtime overhead for users?

james-garner-canonical

I've left some major and minor comments.

Major: I think the CLI interface is pretty nice, but what actually happens with the output could be made more clear. I'd like to see non-destructive default behaviour by default, and I'd also like to be able to dump a Python class to a Juju action/config definition without worrying about charmcraft.yaml at all.

More minor: while reading through the code, I've left a number of suggestions that I think might make some of the logic easier to follow. However ...

Big picture questions:

How much simpler would interacting with Pydantic model classes be if ops-tools could just import Pydantic?
- Additionally, if we're importing a module that defines a Pydantic class, won't we have to have Pydantic installed anyway?
- We could either add Pydantic to our own deps
- Or if we're relying on the user doing --with=pydantic or something, then we could move our Pydantic handling logic to a module that imports pydantic and only import that module as needed. This would have the benefit of letting us fudge around the Pydantic version, but seems more complicated.
I may be wrong about this, but my impression is that we're doing similar class traversal logic in a few different places for different purposes. This is some of the trickier code to follow imo. Would it be worthwhile to centralise this logic and parse the classes we're consuming into an intermediate data structure that has everything we want represented nicely?
- I also had kind of assumed that the core class parsing logic would be in ops (internals), and we'd use it in ops-tools, since we already parse classes for loading actions/config/relation data in ops. That wouldn't work if we went down the route of using Pydantic here, but perhaps something to keep in mind.

Co-authored-by: James Garner <james.garner@canonical.com>

dimaqq

First round of reviews.

dimaqq · 2025-08-12T23:47:30Z

+    with open(args.charmcraft_yaml, 'w') as raw:
+        yaml.safe_dump(charmcraft_yaml, raw)


If we want folks to use this feature, we could either:

advocate that charmers abandon separate config/actions/metadata.yaml (my pref), or

support updating both charmcraft.yaml and metadata.yaml, and autodetect which is present

P.S. I'm all for updating "in place", however we must use a round-trip safe yaml parser/serialiser and have tests to prove it, including YAML with comments in "inconvenient" places. I've run into that with the charm pin updater script, there are plenty of corner cases when key or content is interspersed with comments.

dimaqq · 2025-09-05T06:00:42Z

+    default = None
+    for field in dataclasses.fields(cls):
+        if field.name != name:
+            continue
+        break
+    else:
+        return default


My 2c: if dataclasses doesn't provide a dict, a helper method that builds a dict is the most natural way to go.

dimaqq · 2025-09-05T06:09:47Z

+            exit_code += 2
+        sys.exit(exit_code)
+
+    if args.merge and 'config' in charmcraft_yaml:


question: Is this for 12-factor charms?

Personally I would like to avoid merge like a plague of rats gorged on plagued infested locusts who ate all the plagued corn...

12F could be solved by a stub / base class for options.

dimaqq · 2025-09-05T06:11:12Z

+        config = charmcraft_yaml['config']['options'].update(config['options'])
+    raw_yaml = _insert_into_charmcraft_yaml(raw_yaml, 'config', {'config': config})
+    if actions:
+        if args.merge and 'actions' in charmcraft_yaml:


Are there unit tests and functional / test vector tests for the merge flag?

dimaqq · 2025-09-05T06:13:07Z

+    generated_schema = ops_tools.config_to_juju_schema(config_class)
+    expected_schema = {
+        'options': {
+            'basic-bool': {
+                'type': 'boolean',
+            },


This is great, but omits YAML formatting.

There needs to be a test for verbatim YAML output.
I'm thinking a stability test perhaps (we may change the code, but the output keeps same "canonical" indent).

dimaqq · 2025-09-05T06:14:09Z

+actions:
+    run-backup:
+        description: Backup the database.
+        params:
+            filename:
+                type: string
+                description: The name of the backup file.
+            compression:
+                type: string
+                description: The type of compression to use.
+                default: gzip
+                enum: [gzip, bzip2]
+        required: [filename]
+        additionalProperties: false


I'd like to see a test that looks exactly like this.

PietroPasotti · 2025-09-26T10:52:31Z

this brings back some memories: https://github.com/PietroPasotti/jinx

dimaqq

Partial review

dimaqq · 2025-09-29T07:36:28Z

 # during unit tests, and test_main failures that subprocess out are often
 # difficult to debug. Uncomment this line to get more informative errors when
 # running the tests.
+# When uncommented the test_hook_and_dispatch_with_failing_hook test will fail.


nit could be perhaps written as "test expected to fail if uncommented".

dimaqq · 2025-09-29T07:38:18Z

+        # TODO: ideally, we would look for ConfigBase classes in the charm
+        # module and autoload from there at this point. Leaving this until the
+        # conversation about if & how the generation is done is resolved.
+


I'm not getting the point of this comment.
Specifically, if I were to read it a few years later, I would be missing the context.

Could future work be tracked in GitHub issues or Jira tickets or roadmap items instead?

dimaqq · 2025-09-29T07:52:07Z

+    my_str: str = 'foo'
+    """A string value."""
+
+    my_secret: ops.Secret | None = None


Issues

What are the semantics for optional-Secret-typed fields?

I guess if a value is passed, the a secret with this id (not label) must exist, or load_config() fails.

And I guess on error, same errors = raise|blocked applies, doesn't it?

And if nothing is passed (None, not empty string), then field if None.

This needs to be documented somewhere.

The spec describes Secret fields, but not optional Secret fields.

The implementation remains the same, doesn't it?

option_type = self.meta.config.get(key) # Convert secret IDs to secret objects. We create the object rather # that using model.get_secret so that it's entirely lazy, in the # same way that SecretEvent.secret is. if option_type and option_type.type == 'secret': assert isinstance(value, str) # Juju will have made sure of this. value = model.Secret( backend=self.model._backend, id=value, _secret_set_cache=self.model._cache._secret_set_cache, ) config[attr] = value

The value seems mandatory?

benhoyt · 2025-11-04T10:13:52Z

Tracking issue: #2163

setuptools_scm was pulled in but never used (no git-tag based versioning) — and on modern setuptools it fails to import because it still references the removed pkg_resources.

Two post-merge regressions in ops-tools: 1. _attr_to_yaml_type was treating ops.Secret | None as a multi-type union (keeping NoneType in the hint set) and falling back to 'string'. Strip NoneType so Optional[T] is treated as T, restoring 'type: secret' for optional secret fields. 2. Pydantic 2.12 no longer inlines Enum schemas; it emits $defs + $ref. action_to_juju_schema now calls model_json_schema() (schema() is deprecated) and inlines referenced schemas so the Juju YAML retains type / enum instead of a dangling $ref.

* JSON_TYPES comment wording (james-garner-canonical): tighten the description of what's handled outside the mapping. * test_main charm comment (dimaqq): reword to match the style of the surrounding comment. * _attrdocs unit tests (dimaqq): add two edge cases -- a class docstring must not attach to the first attribute, and a method between attributes must not break docstring collection.

Share the dataclass / Pydantic class traversal with ops.charm rather than reimplementing it here. ops-tools stays permissive by catching the ValueError that _juju_fields raises on plain classes and falling back to get_type_hints.

update-charmcraft-schema requires a charmcraft.yaml to read and write back, which makes it awkward to try the generator interactively, pipe the output somewhere else, or drive it from a charmcraft override-build part (where charmcraft.yaml may not declare config/actions at all). Add two standalone tools that take one or more Python class specifiers and write the matching Juju YAML (config.yaml / actions.yaml format) to stdout. They share get_class_from_module with the existing tool (moved into _generate_juju_yaml.py) but do not read charmcraft.yaml.

tonyandrewmeyer added 30 commits March 29, 2025 19:50

Initial implementation.

4ee7643

Merge origin/main.

da8d265

Tox is green, including docs.

7f143cd

Add extra tests.

739c20c

Tox is green again.

2c9bb9f

Add a Raises section.

a1d4f74

Expand to-do list.

fac6c7c

Adjustments after initial review.

f155bf5

Provide pydantic for static checks.

ea10a4d

Changes from initial review.

bd2a2df

Merge from main.

eb2d6e1

Tweaks after merge.

35b643a

Add TODO.

441c68a

Remove ignore used during dev.

0966028

Add TODO note.

29875b1

Do the status set right at the end, so that it only happens if charms…

f13ed9d

… don't catch the error themselves.

Tweak comment.

4320d97

Use cleaner methods to get the set of fields.

84ba899

Only pass the config that the class defines. This makes it easier to …

b13c742

…break the config into multiple classes.

Provide lazy Secrets in the config - this means you get errors later …

30573f9

…if the secret isn't available, but avoids Juju calls when loading config.

With lazy Secrets, we no longer get errors with missing secrets/inval…

172c692

…id secret IDs.

Clean up the attr-to-type code.

3ee1807

Add a test for the 'only load some config' case.

5f8db50

Move TODO to a better location.

d932a97

Merge origin/main.

80dfa63

Add tests for uncaught InvalidSchemaError.

197431a

Fix docs.

d0b7e2e

Adjustments from spec review.

1c09153

Merge origin/main.

b716878

Provide Python 3.8 support.

97cd70a

james-garner-canonical reviewed Aug 29, 2025

View reviewed changes

Comment thread tools/src/ops_tools/_update_charmcraft_yaml.py Outdated

james-garner-canonical reviewed Aug 29, 2025

View reviewed changes

Comment thread tools/src/ops_tools/_update_charmcraft_yaml.py Outdated

james-garner-canonical reviewed Aug 29, 2025

View reviewed changes

james-garner-canonical requested changes Aug 29, 2025

View reviewed changes

Apply suggestions from code review

3d26c4b

Co-authored-by: James Garner <james.garner@canonical.com>

dimaqq reviewed Sep 5, 2025

View reviewed changes

dimaqq reviewed Sep 29, 2025

View reviewed changes

tonyandrewmeyer requested a review from Copilot November 3, 2025 10:47

This comment was marked as outdated.

Sign in to view

tonyandrewmeyer added 18 commits February 8, 2026 21:20

Remove the --merge and --diff options.

223ed26

Remove unnecessary comment.

7f99709

Include tools in coverage.

04631c2

Tweak comment, per review.

d670d04

Minor readability improvement, per review.

b6a8e58

Minor readability improvement, per review.

0dfbfa2

Minor refactor, per review.

10d06ef

Minor refactor, per review.

f3ef160

Handle lists and tuples in config.

d462027

Align the behaviour of juju_names exactly with Relation.save().

b89c2b5

Merge origin/main.

f7c1747

Merge upstream/main.

34a16de

Drop unused setuptools_scm build dep in ops-tools.

ff0b351

setuptools_scm was pulled in but never used (no git-tag based versioning) — and on modern setuptools it fails to import because it still references the removed pkg_resources.

Delegate juju_names to ops.charm._juju_fields.

89e8acb

Share the dataclass / Pydantic class traversal with ops.charm rather than reimplementing it here. ops-tools stays permissive by catching the ValueError that _juju_fields raises on plain classes and falling back to get_type_hints.

The cycle field isn't needed any more.

1f98af9

tonyandrewmeyer marked this pull request as draft April 17, 2026 01:59

		with open(args.charmcraft_yaml, 'w') as raw:
		yaml.safe_dump(charmcraft_yaml, raw)

Conversation

tonyandrewmeyer commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

james-garner-canonical left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaqq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PietroPasotti commented Sep 26, 2025

Uh oh!

dimaqq left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaqq Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

benhoyt commented Nov 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tonyandrewmeyer commented Aug 11, 2025 •

edited

Loading

james-garner-canonical left a comment •

edited

Loading

dimaqq Sep 29, 2025 •

edited

Loading