canonical / ops-scenario

State-transition testing SDK for Operator Framework Juju charms.
Apache License 2.0
10 stars 7 forks source link

chore!: adjust privacy #176

Closed tonyandrewmeyer closed 2 weeks ago

tonyandrewmeyer commented 3 weeks ago

A collection of small changes that generally fall into "remove x from the public API":

Fixes #175.

tonyandrewmeyer commented 3 weeks ago

Talked in person about these:

  • [ ] Custom exceptions

Keep them, move them to scenario.errors.

  • [ ] Best way to hide consistency_checker

Make the module name _consistency_checker.py, leave the privacy of the methods inside of it alone.

  • [ ] DEFAULT_JUJU_VERSION - just get rid of it?

Or make it _DEFAULT_JUJU_VERSION.

  • [ ] scenario.types?

Avoid this, try to minimise the number but put them at the top so that we don't repeat the from typing import pattern, which we don't like.

  • [ ] DEFAULT_JUJU_DATABAG private or not?

Yes, make it private (maybe move it back where it was to minimise churn). The use-case I had in mind was someone wanting to say "use this different databag as the default" but you can just have a constant in your own charm test code and pass it in each time, and if we do want to let people set the default, it would be better if we designed a proper API around that.

Also discussed next_relation_id. We'd like to remove this - there are some use cases, but it's currently only used in 2.5 charms that I can find (mysql-router, mysql-router-k8s, spark-history-server-k8s), and at the end of the day it's just a convenience - there are other ways to do it.

(The simplest way is next_relation_id = Relation("name of some relation that exists in meta but doesn't matter which one").id - but there are other Pythonic ways to do this too).

tonyandrewmeyer commented 3 weeks ago

Drat, I typed git push right before walking about the door, but my changes aren't here so I guess something failed there. I don't have that device set up for any remote access right now, so this will have to wait for proper review until Monday, sorry.

tonyandrewmeyer commented 2 weeks ago

@PietroPasotti are you good with the changes here?