/srv/irclogs.ubuntu.com/2020/03/12/#smooth-operator.txt

=== jam1 is now known as jam
niemeyerMornings09:17
Chipacagood morning!09:18
Chipacajam: 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-ish09:24
ChipacaI wonder what Facu thinks about two-vs-one, maybe it's not as unpythonic as i think09:28
niemeyerThe main question I'd ask in this context is whether people will use both almost always09:29
niemeyerIf that's true, then returning both seems fine.. if it's not true, then it's a bit awkward09:29
niemeyerThe create_harness name also leans itself to a single argument a bit better09:31
niemeyerAnother advantage of the single argument is that as the harness is passed around, the many test methods will have the charm at hand as well09:32
Chipacahm, mark maglana makes a fair point10:30
Chipacabah, more or less10:30
Chipacahm10:30
jamChipaca: well for me, integration tests would be the next level up, where you are interacting with other charms.10:44
jamI don't see a way to "just do unit tests" of MyCharm.on_install without having the harness10:44
jamsince the whole point is that on_install is going to look at the model and figure out what it needs to do.10:44
Chipacathat's the "more or less" thing: there isn't One True Definition of where the line between unit and integration and functional and … is10:44
Chipacajam: i'll answer on the pr10:46
Chipacahopefully that communicated the nuances10:52
jamthx11:00
jamhttps://github.com/canonical/operator/pull/181 is some docs for charm.py. It turns out I didn't want to make anything private.11:00
Chipacajam: yeah from that ginormous list i grepped there wasn't anything that stood out as private, to my eyes11:01
ChipacaWRT #160, do we want ops to be a namespace or not?11:02
Chipacawe could always have a separate namespace package that's only used for that11:02
Chipacamight make things easier11:02
Chipacae.g. have ops-helpers11:02
Chipacaops_helpers*11:03
Chipacathen 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 itself11:04
jamunfortunately this falls very much into "until we use it in anger, we won't really know how one vs the other feels"11:10
jamops.helpers is a bit nicer, IMO, but requires ops to be a namespace first.11:11
jamChipaca: 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.relations11:12
jamthe other 2 are not directly exposed.11:13
jamHandle is probably also something that is more of an implementation detail for events.11:14
jamIt doesn't feel like something that we would want charmers to actively engage with.11:14
jamChipaca: it is now quite hard for me to go through looking for private things and *not* docstring everything...11:15
jamStoredDict/List/Set also feel like they should be hidden details.11:16
jamYes you have one of them, but you shouldn't be creating them.11:16
Chipacajust because it's exposed doesn't mean it has to be public, right?11:16
Chipacai mean we could document it as a collections.abc.Mapping or something11:17
FacuMuy buenos días a todos!11:22
ChipacaFacu: goooood  morning!11:28
Facu:)11:28
Chipacahehe, i hadn't use that one in *ages*11:29
Chipacaniemeyer: when does your self-immolation^Wisolation end?11:30
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 … is11:38
niemeyerYeah, 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 tests11:39
niemeyerI haven't seen the issue yet, though, so let me read it to get more context11:39
niemeyerChipaca: My current plan is to go back home on Saturday if nothing else happens11:39
niemeyerLike me starting to cough desperately..11:39
Facuhttps://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?11:40
niemeyerHmm.. I don't see any issues related to functional testing11:41
niemeyerFacu: 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 bit11:41
niemeyerFacu: But basically,11:41
niemeyerFacu: 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,11:42
Facu(I was trying to spot the part of the code that implements this kind of communication to juju)11:42
niemeyerFacu: 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, etc11:43
niemeyerFacu: 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 available11:43
niemeyerFacu: With that knowledge, navigate for a while in the model code and you'll get a good grasp of all the pieces11:44
Facuniemeyer, for example, that open_port would be a method of model.Network?11:44
niemeyerFacu: Maybe, but maybe not depending on what that means11:45
niemeyerFacu: 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?11:46
Facummm... or maybe Unit11:46
Facuniemeyer, perfect, thanks11:46
niemeyerFacu: With that questioning, look through the existing model and try to picture it11:46
Chipacaniemeyer: wrt testing, comment at the bottom of the harness pr11:49
niemeyerChipaca: Thanks.. yeah, not really worried about that point11:51
niemeyerIf all people every do is use the harness to test their charm, that's still much better than doing absolutely nothing..11:52
niemeyerWe won't be able to force people to write tests they don't want to, no matter how we name it11:52
niemeyerWe can encourage them to write tests, though, by making it as easy as possible to do so11:52
Chipacasigh. People that are convinced they follow the One True Way.11:54
Chipacathe one true way being, in this case, apparently, "mock all the things"11:54
* Chipaca mocks the 'mock all the things' attitude11:54
Chipacaactually, i'm being mean. i should stop that.11:55
niemeyerYeah, stop mocking them11:55
jamChipaca: 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 anyway11:57
jamI'm not sure there is much we could do on our end.11:57
jamI do understand having clear Interfaces for your code. But eventually the abstractions make it hard to know what is actually going on.11:57
niemeyer> what would it look like if our events passed the context in and got it back out again11:57
jamI personally find the test quite obtuse to actually see what it is setting up and testing.11:57
niemeyerWhat do you mean here?11:57
jamniemeyer: if the State and Model were passed as parameters instead of attributes of the Charm.11:58
jameg, everything is just functional11:58
jambut IMO, that is a different framework.11:58
niemeyerYeah, it's called Haskell11:58
niemeyerWe 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 theories12:00
niemeyerI'll be with you shortly.. will prepare myself a cuppa12:00
ChipacaFacu: show me show me show me :-p12:23
ChipacaFacu: does it involve PrefixedEvents12:24
FacuChipaca, this is what I have https://pastebin.canonical.com/p/P2f8G9c33y/ , focus on lines 18 and 49, which is what we want to change12:24
niemeyer> Facu: show me show me show me :-p12:24
niemeyer> ... how you do that trick ...12:24
niemeyerHopefully not the one that makes me scream, tho12:25
FacuChipaca, 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 MetricWhatever12:25
Chipacaniemeyer: which trick? the muti-coloured one?12:25
niemeyerWe're such a musical group12:25
FacuI 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"12:26
ChipacaFacu: sgtm12:26
niemeyerChipaca: https://www.youtube.com/watch?v=n3nPiBai66M12:26
Chipacaniemeyer: hah! these dots i had not connected12:29
Chipacain fact i think i might have heard this song once a long long time ago12:30
Chipacastrange12:30
ChipacaFacu: 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 it12:33
Chipacathat is, instead of self.on.dumper_relation_joined you do self.on["dumper"].relation_joined etc12:33
Chipacaso that might be a natural way to go there12:33
ChipacaFacu: but you're on the right path wrt not doing this in the void12:34
FacuChipaca, ack, I'll get a real life example working (but before, that summary I promised about diversity and job offer)12:35
ChipacaFacu: ok12:38
jamtesting-harness PR updated for single return value12:41
Chipacajam: what does ‘# noinspection PyProtectedMember’ do?12:43
Dmitrii-Shcharm.on[relation_name].relation_joined would be a good way to do it13:28
Dmitrii-Shyou can't do something like this though: self.framework.observe(charm.on[relation_name].relation_joined, self)13:28
Dmitrii-Shbecause the relation_name part is a charm-specific endpoint and your interface type can't handle that variability13:29
Dmitrii-Shself.framework.observe(charm.on[relation_name].relation_joined, self.on_relation_joined)  works13:29
Chipacai think i'm going to go for a run13:46
FacuI 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)13:47
ChipacaFacu: yep, we reached the same conclusion13:48
Chipacashortly before you joined, we're discouraging the shorthand 'self'13:48
Chipacabecause of what it leads to13:48
Chipacabut, muscle memory :-)13:48
ChipacaFacu: this based in part on feedback from users that, seeing 'self', wonder why there isn't just a 'autoconnect_all_the_things'13:49
FacuChipaca, this means we will not allow this generic/implicit way? or that we would just encourage the other method?13:51
ChipacaFacu: the latter13:51
Facuack13:52
Chipacaactually i think we might want to think of doing the former13:52
Chipacabut for now certainly the latter13:52
* Chipaca postpones the run13:53
FacuI 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" hook15:13
Facuif I destroy the unit and deploy it again it's fine, but takes a loot15:13
niemeyerFacu: See juju resolve15:13
niemeyerFacu: See juju resolved15:13
Facuniemeyer, thanks15:13
Facuniemeyer, 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 anything15:14
niemeyerFacu: It doesn't15:16
niemeyerFacu: The idea is that after units error out they stop completely15:16
Facuniemeyer, perfect, thanks15:16
niemeyerFacu: So you say "go on, you should be good to go now"15:17
FacuI 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/ )15:18
niemeyerFacu: Not hold still, but just don't retry the hook that failed..15:18
niemeyerFacu: It may run other hooks15:18
niemeyerFacu: and the same hook again if it has to for other reasons15:18
Facuack, "install" is a one time only hook, right? where all this is documented?15:19
niemeyerYeah, one time15:20
Chipacajam: is rtype:None the usual way of saying "this returns None"? (asking because None is not a type)16:14
niemeyerThis tastes sooo unpythonic16:19
Chipacaniemeyer: which this?16:28
niemeyerrtype:None16:28
Chipacaheh16:29
niemeyerI assume that's the type hinting, but maybe not?16:29
Chipacait's purportedly picked up by sphynx and presented in nice ways16:29
niemeyerAh, sorry.. ECONTEXT16:29
Chipacai've been making noises about using python's type hinting (which can be picked up by sphynx as well), which reads nicer imho16:29
Chipacabut people seem unconvinced :-)16:30
Chipacaniemeyer: https://github.com/agronholm/sphinx-autodoc-typehints#sphinx-autodoc-typehints is what i meant there16:31
* Chipaca ⇝ cuppa tea17:06
niemeyerAlright!17:09
niemeyerWho wants some reviews!?17:09
Facuniemeyer, I do! https://github.com/canonical/operator/pull/16817:10
FacuI'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...17:16
* Facu needs a rubber duck that actually knows juju and charms17:17
niemeyerFacu: Reviewed.. I think we need to talk about it17:17
niemeyerFacu: But not here.. more bandwidth17:17
Facuniemeyer, https://meet.google.com/veq-yfqm-kdk ?17:17
niemeyerGreat, let's do it17:18
Chipacaniemeyer: you said 'John--' but i'd already closed the window17:49
Chipacaniemeyer: I'm going to assume it was "John you're so awesome" until disproven17:50
niemeyerChipaca: That was it17:51
* Chipaca ← winning17:52
* Chipaca closes ALL the PRs18:03
Chipacaniemeyer: if you have a bit of time, I wonder if you have thoughts on https://github.com/canonical/operator/pull/157#pullrequestreview-370519700-body-html18:06
niemeyerI'll do a full pass before the end of my day, but will take a quick break now18:09
Chipacaniemeyer: also for when you come back, https://github.com/canonical/operator/issues/17818:18
ChipacaFacu: #182 fwiw18:34
Facuchecking18:34
ChipacaFacu: I'd add a static checker to the Makefile but we want less of that arcana, not more :-)18:37
ChipacaOTOH we could put it in the .travis.yml instead of the Makefile :-D18:38
ChipacaOTOH² the 'right' way would involve the ast18:39
FacuChipaca, I'd not worry about that, this stuff is normally solved by code reviews and education18:44
FacuChipaca, not all unpythonicies (?) need to be catched automatically18:44
ChipacaFacu: unpythonicities* :)18:46
ChipacaFacu: also, cue Jasper18:47
FacuChipaca, "Jasper"?18:48
ChipacaFacu: https://www.youtube.com/watch?v=sKiLfH3DVGc18:48
Facuje18:48
ChipacaI really wonder why there isn't a flag on logging to use format instead of C-style18:53
Chipacasomebody should PIP that18:53
FacuChipaca, I have an item in my projects list to try to do that19:08
* Facu -> short errands19:08
* niemeyer returns19:42
* Facu is back19:55
FacuYES! I DID IT!21:09
* Facu does the success dance21:09
FacuI now have my super charm deployed together with postgresql21:09
Facumy super charm and a custom/fake db interface21:10
niemeyerWooahy21:10
Facuhttps://pastebin.canonical.com/p/ftCNRZgwrH/21:10
* niemeyer would high-five, but that's disallowed now21:10
Facujajaj21:10
FacuI'm now in the position of starting to really explore the change21:11
Facubut tomorrow!21:11
* Facu eods21:11
niemeyerThat DBAvailable event really wants me to make StoredState work there21:11
niemeyers/wants me/makes me want/21:12

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!