[09:10] good morning all! [09:12] o/ [09:15] mthaddon: how's things? [09:15] staying healthy so far thanks - and you? [09:16] mthaddon: healthy *and* sane! [09:17] * Chipaca is winning [09:18] :) [09:19] mthaddon: I keep on going back to your 'operator framework documentation' document, and i'm thankful for it every time [09:20] ah cool, yeah, I saw you'd started work on rewriting the README. Glad that's proving useful [09:46] t0mb0: 👋 [11:07] Muy buenos días a todos! [11:08] facubatista: ｇｏｏｏｄｍｏｒｎｉｎｇ！ [11:08] hola Chipaca [11:26] * Chipaca takes a braek [11:51] the Framework.observe method gets whatever we pass it as a callback and checks isinstance(observer, types.MethodType) [11:51] why it has to be a method? [11:52] it exploded to me, I was doing a quick self.framework.observe(self.on.upgrade_charm, lambda e: logger.info("====== upgrade ok")) [11:54] I don't know why, when I saw that I thought "that should be something like `callable`" [11:54] let's see if somebody has a reason for that, otherwise I'll fix it [11:59] facubatista: maybe better filed as a bug, people seem off today [12:04] facubatista: Events are persisted [12:04] facubatista: We don't want to store code in the database [12:05] facubatista: Hopefully the error method was indicative of what it needed to be, or was it not? [12:05] s/error method/error string/ [12:06] niemeyer: I don't understand how one thing follows from the other [12:06] RuntimeError: Observer method not provided explicitly and function type has no "on_upgrade_charm" method [12:07] I understand now "Observer" as I read the framework code [12:07] facubatista: That's not entirely clear [12:07] niemeyer, right [12:07] facubatista: Might be improved indeed [12:07] Chipaca: Which things thing follows what? [12:08] Chipaca, we store the *method name*, not the method itself [12:08] niemeyer: events are persisted, we don't want to store code in the database, so the handler needs to be a method [12:08] Chipaca, later we do a getattr, I guess [12:08] we store the method name? why? [12:08] Chipaca: Deferring [12:09] but we stored the event, and the charm is initialized so all the handlers are hooked up [12:09] Chipaca: we don't call the same observer twice unless it asks to be called again [12:09] why would we store the method name? [12:09] Chipaca: I just responded that one.. :) [12:09] no you didn't :) [12:10] Such a productive conversation [12:10] yes [12:10] you are being obtuse [12:10] please stop [12:11] Chipaca: I've been trying to explain things, and meanwhile you've been saying the equivalent of "I don't understand it".. unless you ask intelligent questions, I can't help you [12:11] niemeyer, if I get an event foo, and I defer it, why can't we store "foo", then in the next run, we deliver "foo" to whatever is hooked at that time? [12:11] facubatista: Because of that issue above: we don't call the same observer twice unless it asks to be called again [12:12] facubatista: For that to happen, we need an identity for that "it asks" [12:12] facubatista: How would you do that for a lambda? [12:12] ok, now I understood, thank you [12:13] but if I deferred it, I'm not asking to deliver me the event in the future? the problem is if I change the method or callable (charm upgrade) in the middle? [12:13] facubatista: but you might have multiple handlers for the event [12:14] facubatista: and only some of them deferred [12:14] ah, right [12:14] niemeyer, Chipaca, thanks! [12:14] this is the kind of details we need to be sure is in the docs :) [12:14] facubatista: and in the errors :-D [12:15] facubatista: To be honest, I've been always double-hearted about the whole deferring thing [12:15] It might not pull its weight [12:16] It's also worth noting in this context that we err on the safe side.. [12:16] Despite the apparent way it works, internally handlers don't really ask for deferring an event [12:16] They do the opposite: the ack the event as handled [12:17] The act of deferring prevents that acknowledgement [12:17] niemeyer, deferring event across runs complicates a lot of little things (not only the code, also the developer thinking of the model) -- are there cases where deferring the event is the only sane option? [12:18] (with "model" I didn't mean the juju model, but the "how my charm works and handle all system complexities") [12:18] This is a long way to say that deferring doesn't in fact trigger a complex path into the code.. we _always_ persist events, and then trigger a common path that handles pending events [12:19] That's what makes me feel a bit less bad about the complexity.. [12:19] Our logic makes it harder for events to be lost, even if you never call defer() at all [12:21] facubatista: I would simply not explain any of that complexity until really required.. if people have to understand the details of that stuff, we failed [12:21] niemeyer, mmm... if we don't persist it, and we crash while handling it... the Agent would not re-send it as we never acked it? [12:22] facubatista: I'm a bit confused about the context for this question.. we always persist it, right? [12:22] (don't really know how the Agent works in this regard) [12:22] facubatista: Yeah, that's what I was trying to explain above: we *always* persist it [12:22] niemeyer, yes, we always persist it; I'm challenging that decision :) [12:22] facubatista: This is not triggered by deferring [12:22] facubatista: The difference in deferring is actually very small in terms of overall behavior [12:23] facubatista: You simply flip a switch saying "do I remove this event now that I handled it?" [12:23] facubatista: Everything else is exactly the same [12:23] niemeyer, yes, because we always persist it [12:23] Yeah [12:23] niemeyer, bear with me a little in this other POV [12:23] facubatista: Which means that on crashes the event will be sent again [12:24] niemeyer, what if we NOT always persist it? what if we persist it only when deferring? we may crash while handling it, yes, but shouldn't the Agent re-send the event? [12:24] facubatista: Not necessarily, and our events don't come only from the agent [12:25] ah, right, it may be a pure internal event [12:25] facubatista: This entire logic works any time something calls emit() [12:27] niemeyer, great, I see now all the picture, thanks! [12:28] * facubatista adds a note to improve that error message [12:32] facubatista: np, and it sounds healthy to continue challenging such options [12:41] niemeyer: question: in what situations do we expect snapshot/restore to be used? [12:42] Chipaca: Hopefully not much.. it's the underlying mechanism that enables StoredState, which is a much nicer interface for normal development [12:43] niemeyer: so https://github.com/johnsca/interface-mysql/blob/master/interface_mysql.py is not kosher then? [12:43] maybe it predates storedstate? [12:45] Chipaca: EventBase is not an Object at the moment.. I had an exchange here with facubatista recently covering that.. he was going to do some research to see how hard it would make it a more usual object [12:45] I think that's the only odd case we have [13:07] I won't be around for the standup today, but please drop me a note here if you need something [13:15] niemeyer, comments on this would be great, thanks! https://docs.google.com/document/d/1zKwzo26bNVFgohaRB_kdUCsFPY3OcjfcpQklGpkOHpc/edit# [13:15] * facubatista brb [13:21] * facubatista is back [14:04] * Chipaca wishes all standups went like that [14:32] ah so this is where the cool kids hang [14:32] hang out [14:32] (is probably better English !) [14:35] hello axino [14:44] mthaddon: https://github.com/canonical/operator/blob/06d242ff31b4a5cc6e7e829f9e8f99c6974def86/tbd_examples.md FWIW [14:44] axino: welcome! [14:44] o/ [14:44] mthaddon: I'm just staging stuff in that PR while I finish getting our docs ducks docked [14:46] Chipaca: nice! definitely like the format, makes sense [14:46] this assumes i understood the questions tho :-D [15:01] facubatista: Replied with a few comments [15:01] facubatista: In general it looks nice [15:01] Hello new people [15:03] niemeyer, ack [15:22] niemeyer, we are in the same page, then; there's one extra case annotated now in the spec, but I need to check with juju one detail about multiple parameters [15:27] and confirmed! [15:28] (multiple --breakpoint parameters is possible in juju) [15:28] there are current cases for this, e.g. --overlay in juju deploy [15:28] I'd just accept --breakpoint=a,b,c as you suggested [15:29] niemeyer, in reality, if the user goes and write --breakpoint=a,b,c, juju would set up JUJU_BREAKPOINT to "a,b,c", and it will work just the same :) [15:29] Right [15:29] As long as you define that this is the behavior :) [15:29] the Spec is in good shape [15:30] yay, my first spec here :p [15:30] 👏 👏 👏 👏 👏 [15:31] facubatista: it's even better than that [15:31] facubatista: it's _the_ first spec here :-D [15:32] jajaja [15:32] * facubatista -> lunch [16:34] * facubatista is back [17:15] hmm [17:15] facubatista: i just set the approved date on that spec, but then thought [17:16] maybe we should ask our juju friends first :) [17:16] Chipaca, what about? [17:16] I mean, ask them? [17:16] facubatista: whether they're ok with the changes to debug-hook [17:17] can't imagine a scenario where they block it though [17:17] * Chipaca might just be short of imagination [17:17] Chipaca, it's a step in the milestone, but it's a good idea indeed to gain consensus in the spec itself [17:18] Chipaca, who would be the persons to contact about this? [17:18] facubatista: I'll see if rick_h has a moment [18:17] facubatista: ok, no answer on that front, so I'd say let's leave it with jam [20:23] YES YES YES [20:23] * facubatista dances the success dance [21:00] * facubatista eods and eows [21:00] see you all on Monday [21:37] aw man i missed the success dance [21:41] Never too late for dancing..