-
Notifications
You must be signed in to change notification settings - Fork 125
Description
In #2123 we excluded the charm_spec attribute from the `ops.testing.Context because:
... although it's public, the type is not, and I don't think we want to encourage its use (I think we would make this private if we could).
I wonder if it's worth deprecating the public charm_spec attribute in a separate PR. You convinced me in the other, now-closed PR that it's worth doing this kind of thing (especially in testing) to steer users in the right direction even if we don't have a timeline for the next major release -- and things that we've prepped with a sufficient deprecation warning period can probably be removed then, regardless of why we're doing the major release.
@tonyandrewmeyer said:
Interesting thought. I can only find two uses:
- grafana-agent-operator - this is using
charm_specto get the charm Class and adjust a private attribute on it. I would have thought the test could just import the charm class and do it directly - thecharm_typeisn't a copy, it's the same class object.- charmcraft (!) - I guess this is a legitimate use (so perhaps we shouldn't deprecate and maybe should document), pulling the container name from the metadata rather than hard-coding it in the test. The metadata isn't available elsewhere (you can get it from the model, but not until you've already called
run). However, from_context() would do this for you in a nicer way, so maybe the right thing is to encourage that. (The test is usingscenarioas the namespace, so maybe it's quite old?).
We should either plan and go ahead with deprecating this attribute:
- starting with at the very least documenting that it is private
- then moving on to adding a deprecation warning to the public attribute
- and helping the existing users move away.
Alternatively, we should bite the bullet and decide that it's part of the public API and document it as such.