[09:11] good morning all! [09:19] why is my TODO box just growing :( [09:41] Chipaca, clearly because you aren't doing anything :) [09:41] :'( [10:06] Chipaca, thoughts on 'read_relation_data' vs 'get_relation_data' ? I was avoiding having it too close to 'relation_get' but maybe putting it at the front is different enough. [10:07] jam: i think get is better because read has me think it's more expensive than just getting something from memory [10:09] Chipaca, np. I'll push up that change. Will you have a chance to look at the updates since you looked last? [10:10] jam: yep! [10:19] jam: what sample charms do we have? [10:20] Chipaca, charms using the operator framework? [10:20] yep [10:20] ones that we point people at when they want to start using it [10:21] even better, the ones we ourselves worked on [10:21] https://github.com/johnsca/charm-gitlab-k8s/ [10:21] https://github.com/tvansteenburgh/charm-kine/blob/master/lib/charm.py was a fairly early version of the framework but still mostly sound [10:22] https://github.com/relaxdiego/charm-k8s-prometheus/ is using it, but probably isn't what I would point people to [10:23] I haven't reviewed https://github.com/gnuoy/ceph-iscsi/ which is also using the operator framework [10:25] thank you [10:25] dmitrii was working on https://github.com/dshcherb/charm-nats-1 and https://github.com/dshcherb/charm-cockroachdb I believe [10:25] i wouldn't call relaxdiego's work something i'd point people at [10:25] (quite the opposite in fact) [10:26] so agreed on that :) [10:26] indeed. I was working on interface-pgsql but was struggling to test it to know that it really was doing what I wanted. Hopefully this eventually gets back around to it. [10:32] jam: tests unhappy [10:33] jam: (need to de-path some things for 3.5) [10:41] yeah, I was worried about that. [10:54] Chipaca, pushed a fix that passes on 3.5 [10:55] jam: 👍 [10:56] so shall I push the button or would you like to? [10:57] jam: sorry, i meant good on fixing it [10:58] jam: am now doing a full pass [10:58] ah, gotcha [10:58] will wait for you [10:58] (working on follow up [10:59] jam: i'll push the button when i'm done (want to go over the pr prescription to make sure it's current?) [11:23] jam: bad news :-( [11:23] jam: lots of silly things, mostly in docstrings [11:23] jam: good news, if those are the things i'm asking for changes on, we're so almost there [11:24] jam: note not all comments are requests-for-changes though, some are just me rambling / musing [11:26] oh dang nearly half eleven and i haven't made the boys do any schoolwork [11:29] jam: also, not mentioned in the comments there, is that enable/disable should raise a stink if they're called before begin() [11:29] Chipaca, should they? it would let you default to disabled events as soon as the charm is instantiated. [11:30] jam: ah, if that works, a'ight :) [11:30] of course it'd work :) [11:30] ok, fair [11:30] Chipaca, it isn't that I need it to, just that it feels odd to go out of my way to prevent it, when it isn't really wrong per se [11:31] jam: yep, got that [11:31] jam: belay my "should raise a stink" comment :-p [11:34] Chipaca, so *I* would like to have harness.begin() return the charm instance, though for now I've gone with you have to access it as an attribute. If you look at the various tests, they're ready to interact with 'charm' after begin() [11:34] thoughts?? [11:35] jam: i see no harm in returning it as well as keeping a copy [11:35] (as long as it's clearly documented that that's what we do, which shouldn't be hard) [11:38] so there isn't a huge difference, it is https://paste.ubuntu.com/p/8tSdcb74s3/ vs https://paste.ubuntu.com/p/Cj4k8n94Gw/ [11:40] anyway, I'll leave that for a future follow up. It did feel like Gustavo had a strong preference that you would access it as an attribute. [11:41] jam: ok [11:42] it's about what feels right and consistent in the api [11:42] having the two-argument builder thing was strange, and there i'd agree for having it as an attribute without a doubt [11:43] having it split out as a separate step that returns it feels much better than the two-arg thing [11:43] the attribute thing is still alright [11:43] ¯\_(ツ)_/¯ [11:43] so, yes, let's leave it for a future iteration [11:45] so for 'charm_dir'.... It is weird to set charm_dir if you are using CharmBase, but it does make sense to set it if you are using a real charm. [11:46] Chipaca, I definitely think that if you have a valid charm_dir, you'd want to use the charm_dir / 'metadata.yaml' [11:46] Chipaca, one thing I really liked about Harness, was that it was in-memory only, and this change breaks that. [11:46] :) well, _can_ break it [11:47] it can still be all in memory, and that's nice [11:47] (if you look at the test suite, I pulled in importlib, pathlib, shutil, sys, and tempfile in order to test the 'metadata.yaml' handling.) [11:47] and it's nice that it falls back to disk, and not touching it if you know what you're doing [11:48] so we can still write tests that are all in memory and super fast because of it, for any and all things we write, while supporting simplicity and DRY for actual charm writers [11:50] Chipaca, it is also a place where we assume the disk layout of charms (that your Charm class is exactly in a subdirectory of the top charm dir) [11:50] jam: yeah, i'd like to change that at some point, but can be a followup [11:51] it is the advised layout, and you can override it. so maybe that's fine. I wonder about taking the charm_dir itself [11:51] it's fine for now and will mostly dtrt [11:51] jam: it _could_ be done the other way around: find the yaml, set charm dir to the dir that holds it [11:51] that is, walk up from the .py looking for it [11:52] but as i say, later [11:52] jam: i do expect charm_dir to be only useful to people actually having a metadata.yaml so there is that (and, we can add it as a keyword argument to override like with yaml) [11:54] Chipaca, so I went ahead and set charm_dir if we read metadata.yaml, I'll push that up. [12:00] Muy buenos días a todos! [12:05] facubatista, good morning and welcome back. [12:05] hola jam :) [12:05] Chipaca, I think I've addressed all your review comments. Thanks for the thorough review. I appreciate it. [12:12] Facu: buen día caeza! [12:13] facundo__: ETOOMANYFACUNDOS [12:13] on the _other_ hand, i do still have an open positon [12:13] hmmmmmm [12:13] Chipaca, internet's not best day [12:14] facundo__: buen día caeza :) [12:14] Chipaca, hola :) === facundo__ is now known as facubatista [12:19] jam: care to update the description and land it? [12:19] facubatista: last chance to review / ask for changes in the testing pr :-D [12:20] Chipaca, land it, too big too old to keep pointing to details, we can adjust details in the future [12:20] facubatista: <3 [12:21] Chipaca, +1 [12:27] Chipaca, landed. [12:27] thanks for your help [12:30] jam, niemeyer, question about "config-changed" event: do juju guarantee that all events are delivered in order and nothing is skipped? like, is there 0.000% chance that events won't arrive out of order, or any could be missed? [12:31] is really a charm able to react to config "deltas" (as it's being talked in #189) or it always should manage current state? [12:31] facubatista, juju intentionally doesn't include data in events, so if the config changes 2x you might get a single 'config-changed' event. But you are guaranteed to get an event as long as the config is different than the last time we told you about it. [12:32] it is generally an 'at-least-once' model with coalescing. [12:32] jam, that is a promise we can rely on? [12:32] put another way, once your config-changed exits, we record the version of config that we told you about. we then watch to see if there is a new version, and will eventually run config-changed until those numbers match. [12:33] we *might* run relation-changed, or some other hook in the mean time. [12:34] jam, I'm trying to understand if this is how "it works in general" or "a solid block we can rely on to build other behaviours" [12:35] facubatista, so for all data that we want to tell the charm about. the associated hook returning successfully is what we use to update the version that the charm has seen. [12:36] so we store "we have shown Charm config version 5", and we decide to fire config-changed if config version != 5. So we may not show you version 6, if you're already at version 8 by the time the charm can run config-changed. [12:36] but we don't update the version the charm has seen until config-changed returns with success. [12:37] jam, where this version number comes from? [12:39] jam, also, how the charm actually "sees" the config? [12:39] (sorry if these questions are too basic, I'm still diving in) [12:40] facubatista, every update to the config (from a user doing 'juju config foo=bar') causes the Version to increment by one. [12:40] the actual version counter is not exposed to the user or charm anywhere. [12:41] the charm 'sees' the config by executing the 'config-get' hook tool that is in $PATH during a hook execution. [12:41] ah, the charm doesn't see the version! that is important [12:41] (_ModelBackend is where we interface with Juju's hook tools) [12:42] jam, config-get is synchronous? the charm process is blocked and keeps running until it gets back the whole config? [12:43] facubatista, so the other aspect is that Juju fixes the content that config-get will return during the lifetime of any hook. [12:43] (once it returns a value, that value is cached for the rest of the hook execution.) [12:44] config-get is synchronous, but that matters less because we want to make sure that a charm calling it at the beginning of a hook, or at the end of a hook sees the same content. [12:44] and *that* is the version that we record the charm as seeing. [12:46] jam, we record that the charm saw version N because it answered ok to config-changed, or because we got a config-get from it? [12:48] facubatista, because it answered ok to config-changed. [12:51] jam, thanks for all the info, I have a better picture of this now [12:51] facubatista, np [12:54] jam, do you have an example at hand of how a complex "config" would look? the response to a config-get, I mean... [12:55] facubatista, config-get returns a YAML of key-value as defined by the charm's config.yaml schema. which can have quite a bit in it. [12:55] eg: https://api.jujucharms.com/charmstore/v5/postgresql-203/archive/config.yaml [12:56] has about 60 config options. [12:56] now, they can't be nested, so it is all top-level keys, with values that might be a string/int/float [12:57] jam, so it could be super big, but not complex as it's not nested and with restricted data types [12:59] thanks [12:59] yep [16:10] * facubatista -> lunch [17:00] * facubatista is back [17:05] facubatista: welcome back :-p [17:05] :) [17:26] niemeyer, Chipaca, I went back and forward regarding the best way to structure this, I like it how it's now, but not 100% happy: https://docs.google.com/document/d/1zKwzo26bNVFgohaRB_kdUCsFPY3OcjfcpQklGpkOHpc/ [17:27] in any case, everything I thought of is there, so we can tune it [18:02] soft EOD from me. TTFN! [18:36] facubatista: Want to have a quick call on it? [19:18] niemeyer, still around? [19:18] * facubatista was doing a little gym [19:21] facubatista: Yeah, just responding to something else, back soon [19:21] niemeyer, I'm here, ping me and we can jump to https://meet.google.com/veq-yfqm-kdk [19:35] Alright, done withat [19:35] with that [19:35] Joining.. [19:36] facubatista: ping [19:36] niemeyer, going [20:11] * facubatista brb [20:28] * facubatista is back [21:43] Chipaca, I'm heavily changing the Debugger spec, I will let you know when it's done [21:43] but will need to be tomorrow [21:43] * facubatista eods