=== jam1 is now known as jam [09:17] Mornings [09:18] good morning! [09:24] 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 [09:28] I wonder what Facu thinks about two-vs-one, maybe it's not as unpythonic as i think [09:29] The main question I'd ask in this context is whether people will use both almost always [09:29] If that's true, then returning both seems fine.. if it's not true, then it's a bit awkward [09:31] The create_harness name also leans itself to a single argument a bit better [09:32] 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 [10:30] hm, mark maglana makes a fair point [10:30] bah, more or less [10:30] hm [10:44] Chipaca: well for me, integration tests would be the next level up, where you are interacting with other charms. [10:44] I don't see a way to "just do unit tests" of MyCharm.on_install without having the harness [10:44] since the whole point is that on_install is going to look at the model and figure out what it needs to do. [10:44] 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 [10:46] jam: i'll answer on the pr [10:52] hopefully that communicated the nuances [11:00] thx [11:00] https://github.com/canonical/operator/pull/181 is some docs for charm.py. It turns out I didn't want to make anything private. [11:01] jam: yeah from that ginormous list i grepped there wasn't anything that stood out as private, to my eyes [11:02] WRT #160, do we want ops to be a namespace or not? [11:02] we could always have a separate namespace package that's only used for that [11:02] might make things easier [11:02] e.g. have ops-helpers [11:03] ops_helpers* [11:04] 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 [11:10] unfortunately this falls very much into "until we use it in anger, we won't really know how one vs the other feels" [11:11] ops.helpers is a bit nicer, IMO, but requires ops to be a namespace first. [11:12] 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 [11:13] the other 2 are not directly exposed. [11:14] Handle is probably also something that is more of an implementation detail for events. [11:14] It doesn't feel like something that we would want charmers to actively engage with. [11:15] Chipaca: it is now quite hard for me to go through looking for private things and *not* docstring everything... [11:16] StoredDict/List/Set also feel like they should be hidden details. [11:16] Yes you have one of them, but you shouldn't be creating them. [11:16] just because it's exposed doesn't mean it has to be public, right? [11:17] i mean we could document it as a collections.abc.Mapping or something [11:22] Muy buenos días a todos! [11:28] Facu: ｇｏｏｏｏｏｄｍｏｒｎｉｎｇ！ [11:28] :) [11:29] hehe, i hadn't use that one in *ages* [11:30] niemeyer: when does your self-immolation^Wisolation end? [11:38] > 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 [11:39] 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 [11:39] I haven't seen the issue yet, though, so let me read it to get more context [11:39] Chipaca: My current plan is to go back home on Saturday if nothing else happens [11:39] Like me starting to cough desperately.. [11:40] 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? [11:41] Hmm.. I don't see any issues related to functional testing [11:41] 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 [11:41] Facu: But basically, [11:42] 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, [11:42] (I was trying to spot the part of the code that implements this kind of communication to juju) [11:43] 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 [11:43] 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 [11:44] Facu: With that knowledge, navigate for a while in the model code and you'll get a good grasp of all the pieces [11:44] niemeyer, for example, that open_port would be a method of model.Network? [11:45] Facu: Maybe, but maybe not depending on what that means [11:46] 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? [11:46] mmm... or maybe Unit [11:46] niemeyer, perfect, thanks [11:46] Facu: With that questioning, look through the existing model and try to picture it [11:49] niemeyer: wrt testing, comment at the bottom of the harness pr [11:51] Chipaca: Thanks.. yeah, not really worried about that point [11:52] If all people every do is use the harness to test their charm, that's still much better than doing absolutely nothing.. [11:52] We won't be able to force people to write tests they don't want to, no matter how we name it [11:52] We can encourage them to write tests, though, by making it as easy as possible to do so [11:54] sigh. People that are convinced they follow the One True Way. [11:54] the one true way being, in this case, apparently, "mock all the things" [11:54] * Chipaca mocks the 'mock all the things' attitude [11:55] actually, i'm being mean. i should stop that. [11:55] Yeah, stop mocking them [11:57] 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 [11:57] I'm not sure there is much we could do on our end. [11:57] I do understand having clear Interfaces for your code. But eventually the abstractions make it hard to know what is actually going on. [11:57] > what would it look like if our events passed the context in and got it back out again [11:57] I personally find the test quite obtuse to actually see what it is setting up and testing. [11:57] What do you mean here? [11:58] niemeyer: if the State and Model were passed as parameters instead of attributes of the Charm. [11:58] eg, everything is just functional [11:58] but IMO, that is a different framework. [11:58] Yeah, it's called Haskell [12:00] 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 [12:00] I'll be with you shortly.. will prepare myself a cuppa [12:23] Facu: show me show me show me :-p [12:24] Facu: does it involve PrefixedEvents [12:24] 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 [12:24] > Facu: show me show me show me :-p [12:24] > ... how you do that trick ... [12:25] Hopefully not the one that makes me scream, tho [12:25] 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 [12:25] niemeyer: which trick? the muti-coloured one? [12:25] We're such a musical group [12:26] 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" [12:26] Facu: sgtm [12:26] Chipaca: https://www.youtube.com/watch?v=n3nPiBai66M [12:29] niemeyer: hah! these dots i had not connected [12:30] in fact i think i might have heard this song once a long long time ago [12:30] strange [12:33] 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 [12:33] that is, instead of self.on.dumper_relation_joined you do self.on["dumper"].relation_joined etc [12:33] so that might be a natural way to go there [12:34] Facu: but you're on the right path wrt not doing this in the void [12:35] Chipaca, ack, I'll get a real life example working (but before, that summary I promised about diversity and job offer) [12:38] Facu: ok [12:41] testing-harness PR updated for single return value [12:43] jam: what does ‘# noinspection PyProtectedMember’ do? [13:28] charm.on[relation_name].relation_joined would be a good way to do it [13:28] you can't do something like this though: self.framework.observe(charm.on[relation_name].relation_joined, self) [13:29] because the relation_name part is a charm-specific endpoint and your interface type can't handle that variability [13:29] self.framework.observe(charm.on[relation_name].relation_joined, self.on_relation_joined) works [13:46] i think i'm going to go for a run [13:47] 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) [13:48] Facu: yep, we reached the same conclusion [13:48] shortly before you joined, we're discouraging the shorthand 'self' [13:48] because of what it leads to [13:48] but, muscle memory :-) [13:49] Facu: this based in part on feedback from users that, seeing 'self', wonder why there isn't just a 'autoconnect_all_the_things' [13:51] Chipaca, this means we will not allow this generic/implicit way? or that we would just encourage the other method? [13:51] Facu: the latter [13:52] ack [13:52] actually i think we might want to think of doing the former [13:52] but for now certainly the latter [13:53] * Chipaca postpones the run [15:13] 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 [15:13] if I destroy the unit and deploy it again it's fine, but takes a loot [15:13] Facu: See juju resolve [15:13] Facu: See juju resolved [15:13] niemeyer, thanks [15:14] 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 [15:16] Facu: It doesn't [15:16] Facu: The idea is that after units error out they stop completely [15:16] niemeyer, perfect, thanks [15:17] Facu: So you say "go on, you should be good to go now" [15:18] 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/ ) [15:18] Facu: Not hold still, but just don't retry the hook that failed.. [15:18] Facu: It may run other hooks [15:18] Facu: and the same hook again if it has to for other reasons [15:19] ack, "install" is a one time only hook, right? where all this is documented? [15:20] Yeah, one time [16:14] jam: is rtype:None the usual way of saying "this returns None"? (asking because None is not a type) [16:19] This tastes sooo unpythonic [16:28] niemeyer: which this? [16:28] rtype:None [16:29] heh [16:29] I assume that's the type hinting, but maybe not? [16:29] it's purportedly picked up by sphynx and presented in nice ways [16:29] Ah, sorry.. ECONTEXT [16:29] i've been making noises about using python's type hinting (which can be picked up by sphynx as well), which reads nicer imho [16:30] but people seem unconvinced :-) [16:31] niemeyer: https://github.com/agronholm/sphinx-autodoc-typehints#sphinx-autodoc-typehints is what i meant there [17:06] * Chipaca ⇝ cuppa tea [17:09] Alright! [17:09] Who wants some reviews!? [17:10] niemeyer, I do! https://github.com/canonical/operator/pull/168 [17:16] 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... [17:17] * Facu needs a rubber duck that actually knows juju and charms [17:17] Facu: Reviewed.. I think we need to talk about it [17:17] Facu: But not here.. more bandwidth [17:17] niemeyer, https://meet.google.com/veq-yfqm-kdk ? [17:18] Great, let's do it [17:49] niemeyer: you said 'John--' but i'd already closed the window [17:50] niemeyer: I'm going to assume it was "John you're so awesome" until disproven [17:51] Chipaca: That was it [17:52] * Chipaca ← winning [18:03] * Chipaca closes ALL the PRs [18:06] 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 [18:09] I'll do a full pass before the end of my day, but will take a quick break now [18:18] niemeyer: also for when you come back, https://github.com/canonical/operator/issues/178 [18:34] Facu: #182 fwiw [18:34] checking [18:37] Facu: I'd add a static checker to the Makefile but we want less of that arcana, not more :-) [18:38] OTOH we could put it in the .travis.yml instead of the Makefile :-D [18:39] OTOH² the 'right' way would involve the ast [18:44] Chipaca, I'd not worry about that, this stuff is normally solved by code reviews and education [18:44] Chipaca, not all unpythonicies (?) need to be catched automatically [18:46] Facu: unpythonicities* :) [18:47] Facu: also, cue Jasper [18:48] Chipaca, "Jasper"? [18:48] Facu: https://www.youtube.com/watch?v=sKiLfH3DVGc [18:48] je [18:53] I really wonder why there isn't a flag on logging to use format instead of C-style [18:53] somebody should PIP that [19:08] Chipaca, I have an item in my projects list to try to do that [19:08] * Facu -> short errands [19:42] * niemeyer returns [19:55] * Facu is back [21:09] YES! I DID IT! [21:09] * Facu does the success dance [21:09] I now have my super charm deployed together with postgresql [21:10] my super charm and a custom/fake db interface [21:10] Wooahy [21:10] https://pastebin.canonical.com/p/ftCNRZgwrH/ [21:10] * niemeyer would high-five, but that's disallowed now [21:10] jajaj [21:11] I'm now in the position of starting to really explore the change [21:11] but tomorrow! [21:11] * Facu eods [21:11] That DBAvailable event really wants me to make StoredState work there [21:12] s/wants me/makes me want/