divide apps

[PietroPasotti]

Typical charm code:

def do_something_with_relation(self, rel:Relation):
    remote_unit_34 = [unit for units in rel.units if unit.app.name == rel.app.name and unit.app.name.endswith("/34")][0]
   

now:

def do_something_with_relation(self, rel:Relation):
    remote_unit_34 = rel.app / 34

This is useful in tests but also in charm code proper.

[dstathis]

:+1:

[tonyandrewmeyer]

I'm really not a fan of using the maths operators outside of their normal meanings. I strongly dislike pathlib's Path / Path, for example, using the division operator to do the opposite of dividing. A lot of other people (but not enough / not the right ones!) agreed with that back when it was being considered, although maybe it's moved the taste of Pythonistas over the years.

I think this is less bad, because at least you can say an app is divided into units, so the division operator makes sense in that context. I also have some sympathy for the argument that division ought to have been ÷ and / left free for other things since it's widely used for other purposes - but it's pretty embedded as "divide" in programming.

There does seem to be other ways to make this simpler:

remote_unit_34 = [unit for units in rel.units if unit.app.name == rel.app.name and unit.app.name.endswith("/34")][0]

For example:

remote_unit_34 = rel.app.get_unit(34)

[benhoyt]

I agree with Tony's concerns here. I definitely have mixed feelings about Path / Path, though I'll admit it's grown on me. However, I think something like app.get_unit(34) is clearer, and allows for more obvious error handling (it seems weird for various custom exceptions to be raised by the division operator).

[PietroPasotti]

I'd also be happy with App.get_unit(34), I thought the / syntax would be intuitive mnemonic sugar because of how unit names look (traefik/2), but I also get the thought that you oughtn't be dividing something that's not a number. It was a bit of a cheeky thing that doesn't quite fit in the rest of ops. will rename it.

[simskij]

I share the concerns but also think the suggested syntax looks really slick when it matches the juju way of describing it.

On the other hand, less cognitive overhead is better, and having to know how Juju visually displays units for it to make sense is probably one of the overheads we could live without.

+1 on getting a shorthand for getting a unit, however. Abstracting away the defensive coding needed for that in the charm sdk seems like a win. 👍🏼

[benhoyt]

We had a team discussion about this today, and we find this confusing and too "cutesy" (and probably only useful in tests). In charm code, the unit name (including /N number) is typically in a variable. If anything, we'd add a method with a name to achieve this, like app.get_unit(n).

[dimaqq]

Some comments from discussion:

1. in the OP, it should probably be rel.name.endswith rather than rel.app.name.endswith
2. I can't imagine literal unit number 34, so the OP proposal would look more like rel.app / idx where idx comes from arguments or the event
3. I kinda like the similarity between traefik/2 and source code as proposed, but I wonder if it's a bit too magic?
4. behaviour if the index doesn't exist should be specified, it's not obvious

😆 just for the kicks 🙃 should rel.app / 0 always raise ZeroDivisionError?