The main reason behind the static pool definition with Statuses as descriptors was the code completion/typing it enabled.
One solution to this would be to add() a status on __setattr__ if not present already.
This would mean __init__ looks like:

self.status = StatusPool(self, skip_unknown=True, auto_commit=self.AUTO_COMMIT)
self.status.workload = Status("workload")
self.status.relation_1 = Status("relation_1")
self.status.relation_2 = Status("relation_2")

While I see that dynamically adding things works just as well, I'm not convinced it's more ergonomic. A downside of doing it this way is that it clutters __init__. Too many lines. But that's just my very personal take on style 😬
Of course, we could late-bind the Pool like:

I'm going to give it some thought.
But thanks for the input! I'll definitely use most of this and publish a new major release.
@rwcarlsen also had some ideas about how to strip this library down, I'll ask him to give this some thought as well.

Another thought on the dynamic-adding versus statically defined:
do we validate the name/tag, to raise if it's not a valid python descriptor? Otherwise we have a weird asymmetry where

self.status.add(Status('foo'))
self.status.foo # works
self.status.add(Status('foo-*123124sdXX []')) 
self.status. ??? # borks

The main reason behind the static pool definition with Statuses as descriptors was the code completion/typing it enabled.

I'm not a fan of designing hacky or magic code to optimise for editor code completion. Firstly because this is python; if we wanted real typing we would use a language with real typing, so we just need to grimace and bear it (ie. write nice pythonic code and deal with the lack of perfect typing). Secondly, I've seen this kind of thing before - it seems really nice to have great autocompletion, until you look back at the unmaintainable code that made it possible. Gotta make a trade off here.

While I see that dynamically adding things works just as well, I'm not convinced it's more ergonomic. A downside of doing it this way is that it clutters init. Too many lines.

How about this - doesn't clutter init:

def build_status_pool(charm):
  pool = StatusPool(charm, skip_unknown=True, auto_commit=True)
  pool.workload = Status("workload")
  pool.relation_1 = Status("relation_1")
  pool.relation_2 = Status("relation_2")
  return pool

class Charm(...):
  def __init__(self, ...):
    self.status_pool = build_status_pool(self)

Of course, we could late-bind the Pool like:

Huh I've never seen that before in python. What's going on there? If this is simply so the pool can be dynamically built at top-level code, does a build_status_pool(charm) -> StatusPool function acheive the same thing without this kind of binding?

do we validate the name/tag, to raise if it's not a valid python descriptor? Otherwise we have a weird asymmetry ...

I say no. There's still the non-syntax sugar method of accessing the status from the pool, and if a dev wants to name a status foo-*123124sdXX [], then that's on them. :P

do we validate the name/tag, to raise if it's not a valid python descriptor? Otherwise we have a weird asymmetry ...

I say no. There's still the non-syntax sugar method of accessing the status from the pool, and if a dev wants to name a status foo-*123124sdXX [], then that's on them. :P

To add more about why this is fine...

Also note that there are two options for setting a name here:

1. static string
2. dynamically

In case 1, static string, the dev has full control and can ensure they set it to something that can be used as an attribute name if they want to use that.

In case 2, dynamically, you're not going to be using the attribute shorthand anyway. eg.

name = some_function_that_returns_a_string()
pool.add(Status(name))
pool.... # cannot access this way because you don't know what `name` is at edit time
getattr(pool, name) # works regardless if `name` is not valid syntax for an attribute name
pool.get_status(name) # also works

As a general comment though, I do agree that auto-completion should not be the primary driver for implementation design - although it is certainly an important consideration.

In general, I think there needs to be some pondering, discussion, and agreement on how exactly we want the user experience/api to evolve before engaging in major implementation mods. Otherwise we'll just be code thrashing unnecessarily and will just settle on something less than optimal because we got worn out writing/reviewing code.

I think at this stage it will benefit from actually using this library in some charms, and iterating on the api based on usage patterns and painpoints that emerge. This early stage of development is really the only opportunity we'll have to make many breaking changes, as nothing is depending on it yet.

There's an adage that says one should write client code first. As in, write PoC code in a client (ie. a charm) how it most makes sense in context, then write the library/api code based on how you found you needed to use it. This is useful because it avoids writing an api that you imagine to be useful and ergonomic, but then discovering it's not so much when actually using it in a real world charm.

This has kind of been done with the example charm, but it's still difficult to simulate real-world use cases from a small example. For example, with the experimenting I've been doing the last couple of days to integrate into a sunbeam charm, it's become apparent that pretty much all statuses will need to be dynamically added to the pool via add_status, so the static class-based declaration of the pool may not actually be that useful perhaps (I could be wrong, but that's just an example).

This is why I'm in favour of drastically reducing the complexity and featureset here, then gradually adding features back in as we find a need (which will include iterating on the api to ensure the new features form a coherent set).

fyi I'm now using this branch to iterate as I experiment with using it. See related patches:

• charm-glance-k8s: https://review.opendev.org/c/openstack/charm-glance-k8s/+/852688
• charm-ops-sunbeam: https://review.opendev.org/c/openstack/charm-ops-sunbeam/+/852796

Closing because we're going to avoid a major rewrite here, in favour of smaller iterations and prototyping elsewhere.