#smooth-operator 2020-03-11
<jlosito> testing bot
<jlosito> 1 more test
<Chipaca> Dmitrii-Sh: welcome!
<Chipaca> ubuntulog3: also welcome
<Chipaca> jlosito: thanks
<jlosito> np
* Chipaca changed the topic of #smooth-operator to: general discussion of canonical's operator framework || github.com/canonical/operator
<Chipaca> phew
<Chipaca> woop, everything works ;-)
 * Chipaca goes to make dinner
<niemeyer> I'd just say "on the operator framework"
<niemeyer> :P
* Chipaca changed the topic of #smooth-operator to: general discussion of the operator framework || github.com/canonical/operator
<Chipaca> niemeyer: like so?
<niemeyer> Thanks
<Chipaca> i'm sure there's a way of just telling chanserv to do that
<Chipaca> instead of op/topic/deop
<Chipaca> anywhoo, dinner-making
<niemeyer> Have fun!
<niemeyer> It's already beer o'clock here..
<niemeyer> and debugging something.. but at this stage any findings are a bonus :)
<Chipaca> no beer here because it's a light pre-yoga dinner
<Chipaca> might have a beer after yoga; can't be _too_ healthy
<niemeyer> I'm sure your zeneth capabilities would be much improved
<niemeyer> Not so sure about balance though
<Chipaca> i could pretend it's about that, but it's just trying to keep my back un-messed-up
<Facu> niemeyer, did we think already about the following situation? 1. the event is deferred, it's `.snapshot` is called; 2. the charm is upgraded, the event code itself changes! 3. the event is restored, a new code needs to handle previous "format"
<Facu> I mean, are we just leaving that consideration to the developer, providing any guide? any structure?
<niemeyer> Facu: Yeah, we don't expect devs to be using snapshot directly very often
<niemeyer> Facu: We have the StoredState mechanism for that
<niemeyer> People can just do _stored = StoredState() at class level
<niemeyer> And only store there the data they want to persist
<Facu> but everytime you define an Event you need to provide its snapshot/restore pairs, right?
<niemeyer> Naming is important precisely to highlight that this is persisted
<niemeyer> (thus the choice of "_stored")
<niemeyer> Facu: Every time you define an event *type* that needs parameters
<Facu> perfect
<niemeyer> Facu: We should probably make EventBase a child of Object, so StoredState would work there as well
<niemeyer> But I haven't thought through the consequences of that
#smooth-operator 2020-03-12
<niemeyer> Mornings
<Chipaca> good morning!
<Chipaca> jam: WRT create_harness returning one arg or two, I think it being two does signal the divide very cearly, but OTOH it feels very un-pythonic and very go-ish
<Chipaca> I wonder what Facu thinks about two-vs-one, maybe it's not as unpythonic as i think
<niemeyer> The main question I'd ask in this context is whether people will use both almost always
<niemeyer> If that's true, then returning both seems fine.. if it's not true, then it's a bit awkward
<niemeyer> The create_harness name also leans itself to a single argument a bit better
<niemeyer> Another advantage of the single argument is that as the harness is passed around, the many test methods will have the charm at hand as well
<Chipaca> hm, mark maglana makes a fair point
<Chipaca> bah, more or less
<Chipaca> hm
<jam> Chipaca: well for me, integration tests would be the next level up, where you are interacting with other charms.
<jam> I don't see a way to "just do unit tests" of MyCharm.on_install without having the harness
<jam> since the whole point is that on_install is going to look at the model and figure out what it needs to do.
<Chipaca> that's the "more or less" thing: there isn't One True Definition of where the line between unit and integration and functional and â¦ is
<Chipaca> jam: i'll answer on the pr
<Chipaca> hopefully that communicated the nuances
<jam> thx
<jam> https://github.com/canonical/operator/pull/181 is some docs for charm.py. It turns out I didn't want to make anything private.
<Chipaca> jam: yeah from that ginormous list i grepped there wasn't anything that stood out as private, to my eyes
<Chipaca> WRT #160, do we want ops to be a namespace or not?
<Chipaca> we could always have a separate namespace package that's only used for that
<Chipaca> might make things easier
<Chipaca> e.g. have ops-helpers
<Chipaca> ops_helpers*
<Chipaca> then we'd have ops_helpers-apt as a repo/pypi entry, with ops_helpers.apt in it, etc., getting the independent component aspect of the namespaced without the worry about random people polluting ops itself
<jam> unfortunately this falls very much into "until we use it in anger, we won't really know how one vs the other feels"
<jam> ops.helpers is a bit nicer, IMO, but requires ops to be a namespace first.
<jam> Chipaca: so LazyMapping is one that I would consider making private, as it is an implementation detail. same for BindingMapping and *maybe* RelationMapping, though the last one is exposed as an attribute from Model.relations
<jam> the other 2 are not directly exposed.
<jam> Handle is probably also something that is more of an implementation detail for events.
<jam> It doesn't feel like something that we would want charmers to actively engage with.
<jam> Chipaca: it is now quite hard for me to go through looking for private things and *not* docstring everything...
<jam> StoredDict/List/Set also feel like they should be hidden details.
<jam> Yes you have one of them, but you shouldn't be creating them.
<Chipaca> just because it's exposed doesn't mean it has to be public, right?
<Chipaca> i mean we could document it as a collections.abc.Mapping or something
<Facu> Muy buenos dÃ­as a todos!
<Chipaca> Facu: ï½ï½ï½ï½ï½ï½ï½  ï½ï½ï½ï½ï½ï½ï½ï¼
<Facu> :)
<Chipaca> hehe, i hadn't use that one in *ages*
<Chipaca> niemeyer: when does your self-immolation^Wisolation end?
<niemeyer> > that's the "more or less" thing: there isn't One True Definition of where the line between unit and integration and functional and â¦ is
<niemeyer> Yeah, I wouldn't get too worried about that either.. it's more about what we're testing and how than how people choose to label those tests
<niemeyer> I haven't seen the issue yet, though, so let me read it to get more context
<niemeyer> Chipaca: My current plan is to go back home on Saturday if nothing else happens
<niemeyer> Like me starting to cough desperately..
<Facu> https://github.com/canonical/operator/issues/179 talks about adding a `self.open_port` that IIUC a way to telling juju to do something; do we have others like that already implemented? example?
<niemeyer> Hmm.. I don't see any issues related to functional testing
<niemeyer> Facu: Yeah, we have quite a bit of them.. they're not all sticking to the same location, though.. depends a bit on context, which you'll heave to think through a bit
<niemeyer> Facu: But basically,
<niemeyer> Facu: We have a backend that deals with calling the actual commands.. so that's the only piece of code that should be doing that job. It's replaceable for testing. Then,
<Facu> (I was trying to spot the part of the code that implements this kind of communication to juju)
<niemeyer> Facu: The Model is the thing that represents the world view in that specific charm and unit.. through it the code can navigate through several aspects such as configuration, units, apps, etc
<niemeyer> Facu: The backend method that calls the commands sticks on one of these places, trying as much as possible to relate to the specific type where the method is made available
<niemeyer> Facu: With that knowledge, navigate for a while in the model code and you'll get a good grasp of all the pieces
<Facu> niemeyer, for example, that open_port would be a method of model.Network?
<niemeyer> Facu: Maybe, but maybe not depending on what that means
<niemeyer> Facu: I'll need to dig down in the model to remember the exact relationships as well.. here is the thing to keep in mind: what is the job of open/close port?  What exact part of the model is being affected by it?
<Facu> mmm... or maybe Unit
<Facu> niemeyer, perfect, thanks
<niemeyer> Facu: With that questioning, look through the existing model and try to picture it
<Chipaca> niemeyer: wrt testing, comment at the bottom of the harness pr
<niemeyer> Chipaca: Thanks.. yeah, not really worried about that point
<niemeyer> If all people every do is use the harness to test their charm, that's still much better than doing absolutely nothing..
<niemeyer> We won't be able to force people to write tests they don't want to, no matter how we name it
<niemeyer> We can encourage them to write tests, though, by making it as easy as possible to do so
<Chipaca> sigh. People that are convinced they follow the One True Way.
<Chipaca> the one true way being, in this case, apparently, "mock all the things"
 * Chipaca mocks the 'mock all the things' attitude
<Chipaca> actually, i'm being mean. i should stop that.
<niemeyer> Yeah, stop mocking them
<jam> Chipaca: so my particular feeling on his charms is that he has written yet-another-framework. Which might be a framework we want to discuss (what would it look like if our events passed the context in and got it back out again). But if he is going to implement yet-another-adapter anyway
<jam> I'm not sure there is much we could do on our end.
<jam> I do understand having clear Interfaces for your code. But eventually the abstractions make it hard to know what is actually going on.
<niemeyer> > what would it look like if our events passed the context in and got it back out again
<jam> I personally find the test quite obtuse to actually see what it is setting up and testing.
<niemeyer> What do you mean here?
<jam> niemeyer: if the State and Model were passed as parameters instead of attributes of the Charm.
<jam> eg, everything is just functional
<jam> but IMO, that is a different framework.
<niemeyer> Yeah, it's called Haskell
<niemeyer> We want to be following good practices, but we don't want to do so just because it's dogmatic.. it's more relevant to make Python developers feel *happy* to be using our APIs than it is to make a scientist happy that we're using his theories
<niemeyer> I'll be with you shortly.. will prepare myself a cuppa
<Chipaca> Facu: show me show me show me :-p
<Chipaca> Facu: does it involve PrefixedEvents
<Facu> Chipaca, this is what I have https://pastebin.canonical.com/p/P2f8G9c33y/ , focus on lines 18 and 49, which is what we want to change
<niemeyer> > Facu: show me show me show me :-p
<niemeyer> > ... how you do that trick ...
<niemeyer> Hopefully not the one that makes me scream, tho
<Facu> Chipaca, but to really try this I need something "on the other end", I was thinking about installing this charm together with https://jaas.ai/u/ubuntu-ci-engineering/graphite/trusty/1 , and change that FileLogger fake thing with a MetricWhatever
<Chipaca> niemeyer: which trick? the muti-coloured one?
<niemeyer> We're such a musical group
<Facu> I do not want to work "in isolation", I want to have real life examples to have a better grasp on how the changes would "feel"
<Chipaca> Facu: sgtm
<niemeyer> Chipaca: https://www.youtube.com/watch?v=n3nPiBai66M
<Chipaca> niemeyer: hah! these dots i had not connected
<Chipaca> in fact i think i might have heard this song once a long long time ago
<Chipaca> strange
<Chipaca> Facu: wrt PrefixedEvent (and thanks to Dmitrii-Sh for pointing this out) means you do self.on["dumper"] and you get a PrefixedEvent that has non-prefixed events on it
<Chipaca> that is, instead of self.on.dumper_relation_joined you do self.on["dumper"].relation_joined etc
<Chipaca> so that might be a natural way to go there
<Chipaca> Facu: but you're on the right path wrt not doing this in the void
<Facu> Chipaca, ack, I'll get a real life example working (but before, that summary I promised about diversity and job offer)
<Chipaca> Facu: ok
<jam> testing-harness PR updated for single return value
<Chipaca> jam: what does â# noinspection PyProtectedMemberâ do?
<Dmitrii-Sh> charm.on[relation_name].relation_joined would be a good way to do it
<Dmitrii-Sh> you can't do something like this though: self.framework.observe(charm.on[relation_name].relation_joined, self)
<Dmitrii-Sh> because the relation_name part is a charm-specific endpoint and your interface type can't handle that variability
<Dmitrii-Sh> self.framework.observe(charm.on[relation_name].relation_joined, self.on_relation_joined)  works
<Chipaca> i think i'm going to go for a run
<Facu> I don't like the implicitness of that "self" as second parameter, as targets of the events; I think we should encourage people to be explicit there, it's simpler to learn and see each others code (and *maybe* leave that option as an "advanced mechanism", for flexibility)
<Chipaca> Facu: yep, we reached the same conclusion
<Chipaca> shortly before you joined, we're discouraging the shorthand 'self'
<Chipaca> because of what it leads to
<Chipaca> but, muscle memory :-)
<Chipaca> Facu: this based in part on feedback from users that, seeing 'self', wonder why there isn't just a 'autoconnect_all_the_things'
<Facu> Chipaca, this means we will not allow this generic/implicit way? or that we would just encourage the other method?
<Chipaca> Facu: the latter
<Facu> ack
<Chipaca> actually i think we might want to think of doing the former
<Chipaca> but for now certainly the latter
 * Chipaca postpones the run
<Facu> I deployed my charm, which crashed during the install hook call; I fixed it but I can't upgrade it, as it says  unit-my-super-charm-3: 12:11:26 INFO juju.worker.uniter awaiting error resolution for "install" hook
<Facu> if I destroy the unit and deploy it again it's fine, but takes a loot
<niemeyer> Facu: See juju resolve
<niemeyer> Facu: See juju resolved
<Facu> niemeyer, thanks
<Facu> niemeyer, IIUC I should tell it to no retry, as before I need to upgrade it, right? I mean, it doesn't auto-upgrade it or anything
<niemeyer> Facu: It doesn't
<niemeyer> Facu: The idea is that after units error out they stop completely
<Facu> niemeyer, perfect, thanks
<niemeyer> Facu: So you say "go on, you should be good to go now"
<Facu> I used --no-retry=True (which would mean something like "please get out of the error state, but hold still") and then upgraded the charm again (which succeeded!! \o/ )
<niemeyer> Facu: Not hold still, but just don't retry the hook that failed..
<niemeyer> Facu: It may run other hooks
<niemeyer> Facu: and the same hook again if it has to for other reasons
<Facu> ack, "install" is a one time only hook, right? where all this is documented?
<niemeyer> Yeah, one time
<Chipaca> jam: is rtype:None the usual way of saying "this returns None"? (asking because None is not a type)
<niemeyer> This tastes sooo unpythonic
<Chipaca> niemeyer: which this?
<niemeyer> rtype:None
<Chipaca> heh
<niemeyer> I assume that's the type hinting, but maybe not?
<Chipaca> it's purportedly picked up by sphynx and presented in nice ways
<niemeyer> Ah, sorry.. ECONTEXT
<Chipaca> i've been making noises about using python's type hinting (which can be picked up by sphynx as well), which reads nicer imho
<Chipaca> but people seem unconvinced :-)
<Chipaca> niemeyer: https://github.com/agronholm/sphinx-autodoc-typehints#sphinx-autodoc-typehints is what i meant there
 * Chipaca â cuppa tea
<niemeyer> Alright!
<niemeyer> Who wants some reviews!?
<Facu> niemeyer, I do! https://github.com/canonical/operator/pull/168
<Facu> I'm kind of lost about trying a charm and an interface at the same time, probably missing some concepts here and there, I'm kind of stuck...
 * Facu needs a rubber duck that actually knows juju and charms
<niemeyer> Facu: Reviewed.. I think we need to talk about it
<niemeyer> Facu: But not here.. more bandwidth
<Facu> niemeyer, https://meet.google.com/veq-yfqm-kdk ?
<niemeyer> Great, let's do it
<Chipaca> niemeyer: you said 'John--' but i'd already closed the window
<Chipaca> niemeyer: I'm going to assume it was "John you're so awesome" until disproven
<niemeyer> Chipaca: That was it
 * Chipaca â winning
 * Chipaca closes ALL the PRs
<Chipaca> niemeyer: if you have a bit of time, I wonder if you have thoughts on https://github.com/canonical/operator/pull/157#pullrequestreview-370519700-body-html
<niemeyer> I'll do a full pass before the end of my day, but will take a quick break now
<Chipaca> niemeyer: also for when you come back, https://github.com/canonical/operator/issues/178
<Chipaca> Facu: #182 fwiw
<Facu> checking
<Chipaca> Facu: I'd add a static checker to the Makefile but we want less of that arcana, not more :-)
<Chipaca> OTOH we could put it in the .travis.yml instead of the Makefile :-D
<Chipaca> OTOHÂ² the 'right' way would involve the ast
<Facu> Chipaca, I'd not worry about that, this stuff is normally solved by code reviews and education
<Facu> Chipaca, not all unpythonicies (?) need to be catched automatically
<Chipaca> Facu: unpythonicities* :)
<Chipaca> Facu: also, cue Jasper
<Facu> Chipaca, "Jasper"?
<Chipaca> Facu: https://www.youtube.com/watch?v=sKiLfH3DVGc
<Facu> je
<Chipaca> I really wonder why there isn't a flag on logging to use format instead of C-style
<Chipaca> somebody should PIP that
<Facu> Chipaca, I have an item in my projects list to try to do that
 * Facu -> short errands
 * niemeyer returns
 * Facu is back
<Facu> YES! I DID IT!
 * Facu does the success dance
<Facu> I now have my super charm deployed together with postgresql
<Facu> my super charm and a custom/fake db interface
<niemeyer> Wooahy
<Facu> https://pastebin.canonical.com/p/ftCNRZgwrH/
 * niemeyer would high-five, but that's disallowed now
<Facu> jajaj
<Facu> I'm now in the position of starting to really explore the change
<Facu> but tomorrow!
 * Facu eods
<niemeyer> That DBAvailable event really wants me to make StoredState work there
<niemeyer> s/wants me/makes me want/
#smooth-operator 2020-03-13
<Chipaca> good morning peeps!
<niemeyer> Moin moin
<niemeyer> Chipaca: Didn't get to do a full round of reviews yesterday.. thankfully friends/family called
<niemeyer> Got through the harness one, though, and just replied to your first requested point above
<Chipaca> niemeyer: glad you got some social interaction in :-)
<Chipaca> i went out and had a belated birthday dinner with my uncle and my brother+sister-in-law
<Chipaca> we tried not to cough too much at people
<Chipaca> now i'm going to finish the HR training thing before they get upset
<niemeyer> Replied to #178 as well
<Dmitrii-Sh> Thanks for the replies on #157. I made a comment about the "default binding" term here https://github.com/canonical/operator/pull/157#discussion_r392150027 - I hope the example makes sense
<niemeyer> Replied
<Chipaca> i need to step out for a bit
 * Chipaca realises he hasn't actually moved
<Facu> Muy buenos dÃ­as a todos!
<Chipaca> Facu: o/!
<niemeyer> \o
<niemeyer> I'm going to skip the standup today as I'm researching something for a call with Mark in a bit.. I've been reviewing and will be reviewing is my main action points
<Facu> niemeyer, (no rush, just don't want to drop your comment from yesterday) -- "That DBAvailable event really wants me to make StoredState work there" <-- this means to provide to events the same mechanism of "store whatever you want, then access it later" for the charms, right?
<niemeyer> Facu: I meant supporting a _stored = StoredState() attribute in events so people don't need to implement the methods
<Facu> niemeyer, having "one mechanism to rule them all" sounds nice, not having to learn two things... however, Events may be simpler than that, we could have the responsability of spawn the same event using the original information automatically, no developer code needed
<Facu> we could just "freeze" the events and melt them later... we want for Events to not carry around any extra information, right?
<Facu> what about defining the events like this? https://pastebin.canonical.com/p/yzbpvNMKHW/
<niemeyer> Facu: The thinking behind StoredState is that it enables people to very clearly define things they do want persisted and things they don't want persisted
<Facu> we can take care of having those fields inside, saving them and loading them
<niemeyer> Facu: There's no ambiguity when one is using code like self._stored.private_addresss about where that value is coming from, where it goes to
<niemeyer> Facu: This already works for anything that is an Object in the framework world
<niemeyer> Facu: So this need is not unknown to us, or something we need a different design for I think
<Facu> niemeyer, I get that; my point is if we want to encourage people to have "super simple event objects that may carry one or two well-defined pieces of information", or "a generic object with complex machinery that may save some state and flies around"
<niemeyer> Facu: If we do change it, we need to change everything else with it so we don't have diverging patterns inside the same logic
<niemeyer> Facu: Yeah, that's what I'm talking about too
<niemeyer> Facu: I don't see self._stored.private_address as worse than self._private_address + a list of fields elsewhere
<niemeyer> Facu: The list of fields means you'll need to read the list of fields again every time you want to know which of the 10 attributes are persisted or not
<niemeyer> Seems very error prone and unfriendly
<Facu> all those fields are persisted
<niemeyer> Facu: All the fields in the list of fields are persisted
<niemeyer> Facu: All the fields not in the list of fields are not persisted.. right?
<Facu> there are not "fields not in the list", we don't allow them, we enforce the Events to be dead simple
<Facu> that pastebin I shown holds *all the code* for the Event the user needs to write (it was not just the beginning)
<niemeyer> Facu: So what happens when one needs any kind of logic in the event?  Then the dead simple becomes painful?
<niemeyer> Facu: Again.. we have a solution for this problem, because it's a common problem. If our solution needs further improvements, let's do so for everything.
<Facu> we want developers to add logic to the event? if yes, let's go with developers handling what they store by hand, yes
<niemeyer> Facu: If we want people to be able to use normal Python code in their types?  Yes
<niemeyer> Facu: I hope you can see the value in us defining a strong pattern that solves that kind of issue nicely and consistently
<niemeyer> Facu: Does StoredState work in events today?
<Chipaca> niemeyer: standup?
<niemeyer> 11:42:15 <niemeyer> I'm going to skip the standup today as I'm researching something for a call with Mark in a bit.. I've been reviewing and will be reviewing is my main action points
<Chipaca> niemeyer: missed that. thanks.
<Chipaca> niemeyer: good luck with the call
<niemeyer> np, and thanks
<jam> Chipaca: no, its just me trying it out. ":return: None" is probably better.
<Facu> niemeyer, yes, I totally see the value, my other proposal was a way to super restrict users in what they could do with the events, if we wanted that
<Facu> niemeyer, I would need to explore if StoredState works in events today; without exploring/seeing code I don't envision how we could restore/unfreeze an Event that was deferred and make external code to access what the event has stored
<niemeyer> Facu: Restoring data is exactly what StoredState does.. both deferred Events and StoredState use the normal persistence system of the framework
<niemeyer> Things are not so far apart as they may seem
<niemeyer> It probably already works if we make EventBase a child of Object
<Facu> let me play with the code a little
<Facu> this is weird, I just noticed that the timestamps from "juju debug-log" are local (GMT-3), not UTC (which is how the units are configured)
<Facu> which makes me wonder... when I see
<Facu> unit-my-super-charm-5: 11:13:56 INFO juju.worker.uniter.relation joined relation "my-super-charm:dumper postgresql:db"
<Facu> ...that means that the event happened at minute 13 second 56, or that juju is showing me the message at that time?
<Dmitrii-Sh> https://github.com/canonical/operator/pull/157#discussion_r392289930
<niemeyer> Facu: I don't know the response to that.. It would be nice if what you see _locally_ in debug-log is in the local timestamp, though
<niemeyer> Facu: I believe the data is stored in UTC, because that's what the db driver will do by default
<niemeyer> Facu: But.. don't trust me :)
<Facu> so it probably shows me the original timestamp, shifted for my local timezone
<niemeyer> Dmitrii-Sh: As I said there, we seem to be in very different wavelengths
<niemeyer> Dmitrii-Sh: I am (and Chipaca is) trying to explain things in a way that makes sense to users
<niemeyer> Dmitrii-Sh: Saying that we're returning an "app endpoint binding" doesn't make any sense in this context
<niemeyer> Dmitrii-Sh: The API allows people to bind specific spaces to endpoint names, and to specific relations established
<niemeyer> Dmitrii-Sh: That's the general framework people will have in mind when they're coding or using the system
<niemeyer> Dmitrii-Sh: The terminology we pick needs to make sense inside that framework
<niemeyer> Facu: I *hope* so
<Facu> niemeyer, good enough
<Dmitrii-Sh> niemeyer: I think you are right about naming. My suggestion lacks an adjective for "non-relation-specific" bindings.
<Dmitrii-Sh> You could have two apps related via their relation endpoints bound to different spaces as of today AFAIK. It doesn't make a lot of practical sense in my view but relation instances themselves cannot be bound to spaces.
<Dmitrii-Sh> i.e. one can't say
<Dmitrii-Sh> bind "1=myspace"
<Dmitrii-Sh> for relation_id=1
<niemeyer> I don't think I understand what you're trying to say... we accept a relation instance in that method precisely because people can bind spaces to specific relation IDs.. network-get takes an ID parameter doesn't it?
<Dmitrii-Sh> niemeyer: network-get accepts a -r <id> parameter, however, juju doesn't allow things like `juju bind <relation-id>=<space>` or `juju relate <appA> <appB>` via a space
<niemeyer> Dmitrii-Sh: Sorry, can we focus on that first part for a moment
<niemeyer> Dmitrii-Sh: What does that mean that network-get accepts a relation ID?
<Dmitrii-Sh> One can use `network-get <endpoint-name>` or `network-get -r <rid> <endpoint-name>` https://github.com/juju/juju/blob/juju-2.6.7/worker/uniter/runner/jujuc/network-get.go#L83
<Dmitrii-Sh> -r <rid> could only provide a different output in the case of CMR
<niemeyer> Dmitrii-Sh: If you may get a different binding when you provide an ID, that means a relation instance may in fact be bound to a particular space, right?
<Dmitrii-Sh> Not at the moment, a relation id only affects the selection of ingress-address and egress-subnets when CMR is used. It's difficult to explain because the logic is convoluted at the Juju side:
<Dmitrii-Sh> https://github.com/juju/juju/blob/juju-2.7.4/apiserver/facades/agent/uniter/networkinfo.go#L142-L155
<Dmitrii-Sh> https://github.com/juju/juju/blob/juju-2.7.4/apiserver/facades/agent/uniter/networkinfo.go#L310-L334
<niemeyer> Dmitrii-Sh: Man.. can you please stop sending me links? :)
<Dmitrii-Sh> OK, simply put: as a user, I cannot specify a space per relation ID - only per endpoint name.
<niemeyer> Dmitrii-Sh: We're documenting the juju CLI
<niemeyer> Dmitrii-Sh: We're documenting the operator framework API
<niemeyer> Dmitrii-Sh: network-get takes a parameter which is a relation ID
<niemeyer> Dmitrii-Sh: Your code accepts a parameter which is a relation instance.. the method name is get_binding
<niemeyer> Dmitrii-Sh: This is a very simple story.. the confusion comes if we try to reject what is clear
<niemeyer> Dmitrii-Sh: We're NOT documenting the juju CLI
<niemeyer> The alternative here is to change the API so that we don't do any of that
<niemeyer> Taking details out of the binding information
<niemeyer> I don't know how to do that cleanly, though, because that's a "network binding", and these are network binding details
<niemeyer> If we take egress,  ingress, etc out of the network binding details, where do we put them?
<Dmitrii-Sh> not `Relation`s because they might not exist at a given point in time
<Dmitrii-Sh> As an idea, since apps have endpoints: self.model.app.<something>
<niemeyer> How about this: specific relation instances may have different network bindings
<Dmitrii-Sh> so, relation.binding.network.ingress_address would only work if you had a reference to a relation
<Dmitrii-Sh> but get_binding(relation_name) would give you a relation-less binding
<niemeyer> Dmitrii-Sh: We might also make get_binding accept a relation instance
<Dmitrii-Sh> well, then we are back to the naming conversation
<niemeyer> !!!
<Dmitrii-Sh> ok, sorry, I didn't get your point:
<Dmitrii-Sh> 1) relation.binding - OK, we could do that
<Dmitrii-Sh> 2) get_binding(relation_instance) - OK, it's in the framework already
<Dmitrii-Sh> what's the catch?
<niemeyer> The catch is this is a long proposal to change something that apparently works, matches what juju does, and is relatively clean, without a rationale
<niemeyer> We got into this conversation because I was proposed three lines of documentation to explain what was going on
<niemeyer> You disagreed.. and I asked: okay, we have these three scenarios in this method.. how do we document them?
<niemeyer> Which then got you to argue with dozens of links :) about how we don't relation-specific bindings
<niemeyer> *don't have
<niemeyer> Except, well, apparently we do otherwise we cannot explain the behavior
<niemeyer> So.. let me know how you want to document it cleanly :)
<Dmitrii-Sh> ok, I'll think about an alternative for "default binding for that relation" phrasing because it overlaps with "a default binding for all endpoints of an application". I'll suggest it if I come up to something.
<niemeyer> Dmitrii-Sh: "default for the relation" and "default for the application" only overlap in the "default" part.. default is a very common term that has an important meaning, and is hard to replace
<Dmitrii-Sh> I agree, I'd be confused though because "default for the relation" is not documented in the Juju docs. I am not suggesting "default binding" is well documented in Juju either but at least it's mentioned :^)
<Dmitrii-Sh> Either way, replacing a docstring is easier than replacing an API.
<niemeyer> @Dmitrii-Sh Which is exactly why I outlined the three points in my comment in the PR
<niemeyer> Full circle now.. we have bindings for a relation name, bindings for a relation instance that may return the bindings for that instance, and bindings for a relation instance that actually return the bindings assigned to the relation name
<niemeyer> The third case is a fallback
<niemeyer> An easy way to explain a fallback is with the "default" word
<Facu> Ok, I can not use my system's autopep8 because it seem it's too old
<Facu> I'll use a venv, with system-site-packages so I get PyYAML from the system, which is what we wanted to do
<Facu> however, now all the test fails because (for example) files inside the env doesn't have a copyright header :p
<Facu> (I also had to move my test charm out of this directory for the same reason)
<Facu> Chipaca, any objection that we specify "ops" and "test" in the Makefile commands? otherwise it's too intrusive, not letting anybody have other stuff in the same dir while developing
<Dmitrii-Sh> niemeyer: ok, I think we can settle on "default" for now
<Dmitrii-Sh> thanks for guiding me through the thought process behind it
<Chipaca> Facu: and setup.py i guess :-)
<Chipaca> Facu: and â¦ anything else?
<Facu> Chipaca, setup.py already specify     packages=["ops"],
<Chipaca> Facu: i mean the other way around
<Chipaca> Facu: setup.py should be held to the same standard as the rest of the code
<Facu> ah
<Facu> would you mind to explain me the rationale of using *both* autopep8 and flake8?
<Facu> if we're trying to have our code compliant to pep8, have a flake8 guardian should be enough
<Chipaca> Facu: i'm not familiar enough with the details of those two to know what they cover
<Chipaca> or how much overlap they have
<niemeyer> I can explain..
<niemeyer> Facu: We want to stick to flake8 as it's probably the most common tool use for this and it does a good job
<niemeyer> Facu: We use autopep8 to automatically do that for us
<Facu> yes, everybody uses it
<niemeyer> Facu: The goal of using both is to ensure that we remain compatible with flake8, but we want to get the hints that autopep8 give us to tell how to fix it
<niemeyer> *gives us
<niemeyer> That's why autopep8 comes first as well
<niemeyer> And that's why travis runs it with --diff
<niemeyer> Instead of telling us "You guys suck", it says "You guys suck in those specific ways, and this is how you should not suck"
<Facu> ack
<Facu> would you mind if I run these two as part of the unit test infrastructure (together with the grep and find for copyright and backslashes)?
<Facu> that would allow us to have a more simple indication on where everything must run, and get rid of the Makefile
 * Facu would leave just a "test" script 
<niemeyer> We don't need to run both all the time.. I'd run just autopep8 which is the one that gives us more details
<niemeyer> Keep flake8 in travis as a double check
<Facu> I can do that
<niemeyer> Running it in test seems fine, as long as it doesn't take a lot of time to run every time, and that there's a flag to not run it (sometimes you want to test code even if it's messy for the moment)
<Facu> niemeyer, that's one beneffit of getting it as a unit test, you can have a total messy code, and you will only get one failing test (currently it just blocks the execution of the real tests)
<niemeyer> I see what you mean, yeah, sounds good
<Facu> also I'll try also flake8 locally, and see if it's slow or not
<Facu> A mate, and then the final stretch
<niemeyer> I'm envy
 * Facu eods and eows!
<Facu> see you all on Monday!
<niemeyer> Have a good weekend!
<Facu> niemeyer, same for you!
