[08:04] mo'in! [08:09] Morning Chipaca, greetings all [08:13] laptop is once again stuck at 800MHz, I'm going to have to take it apart and unplug the battery again :-| [08:13] probably on the weekend [08:42] so, to support dispatch being a script that calls us, while still expecting us to actually *handle* the dispatch, we need to drop looking at argv to know whether we're dispatch and just look at the env var [08:43] to me that means we should set an env var, or clear an env var, to catch us if we try to exec ourselves again (e.g. if somebody's copied charm.py instead of symlinking it) [08:43] this does not seem too bad to me [08:44] Chipaca: We can't drop the former approach for compatibility, but yeah, we need to make it configurable so that the call site for dispatch can use the env [08:44] niemeyer: 'the former approach', you mean looking at argv when we're _not_ dispatch? [08:44] that seems fine [08:44] Chipaca: Yeah, what we have atm [08:46] niemeyer: on a completely separate track, i'm waiting to hear back but it's likely the 'ops' package on pypi is not considered 'abandoned'. If that's the case, what's our fallback? [08:54] Chipaca: We can register something related and still keep the Python module name [08:55] Chipaca: We should circle back to see how to press further on obsolescence of the name, though [08:55] Chipaca: It's certainly abandoned for long [08:56] niemeyer: pypi has a definition of abandoned that is not just 'has it seen a release' [08:56] working on it, in any case [08:56] Chipaca: Sure, but that seems like a valid case of that for all metrics I can imagine [09:27] Issue operator#240 opened: Shared StoredState between kubernetes workload pods [09:27] niemeyer, Chipaca : maybe they use the metric "has it been installed lately" and most of us have accidentally installed it recently :) [09:33] that is one of the things they look at, yes [09:35] Chipaca: Dmitrii-Sh and I have found this bug: https://github.com/canonical/operator/issues/240 [09:36] * Chipaca looks [09:37] youch [09:39] davigar15: out of ignorance, what creates the symlink from agents//charm to agents//charm ? [09:39] is that juju itself? [09:39] niemeyer, jam, ^^ [09:40] Chipaca, most likely Juju, sounds like on K8s every unit shares the charm dir, which we didn't expect [09:41] jam: isn't it fairly common to use the charm dir to store stuff, like keys and the like? [09:42] Chipaca, so in other discussions I've had, you *shouldn't* use the charm_dir, we've discussed trying to make it read-only in Juju, but I do think it gets uses [09:42] used [09:43] jam: so we're doing something we know we shouldn't? [09:44] Chipaca, I don't think we've expressly discussed it, but some things like "why does the state sometimes end up in the application pod" is because we put stuff in the charm dir, which Juju *also* manages with things like charm upgrades. [09:45] Chipaca, I don't think from Juju's end we've clearly stated to charms "here's the directory you should use for your runtime files" which means they use $PWD which is charm_dir [09:45] jam: yeah, i was about to ask, how _can_ a charm grab a directory for its own use? [09:46] Chipaca, ultimately the charm is the root authority on the machine it is setting up. so it is mostly that we should have a well defined recommendation [09:46] it gets slightly hairier if you install 2 apps to the same machine, who 'wins'. And on K8s there is only 1 operator which is responsible for all charm units. [09:50] Issue operator#241 opened: _TestingModelBackend incorrectly raises KeyError on missing relation [09:52] PR operator#242 opened: Stop test harness relation_ids raising KeyError [10:02] anyway, not really here, just passing by, heading back to other activities [10:02] Chipaca, #241 should be fixed by #196, IIRC [10:02] Issue #241: _TestingModelBackend incorrectly raises KeyError on missing relation [10:02] PR #196: Factor out the Harness changes for test_model.py [10:19] niemeyer: could you re-review #196 ? i think jam addressed your points [10:19] PR #196: Factor out the Harness changes for test_model.py [10:23] davigar15: is that bug blockig your work? [10:25] Chipaca: No, I have a workaround for it :-) [10:25] davigar15: ok [10:25] then i'm going to take a bit of time out to see why my battery is draining despite being plugged in (and my cpu stuck at 800mhz) [10:25] * Chipaca bbiab [10:26] Basically I needed to update the state of the unit with the data retrieved from the relation_data, and then emit an event. Since it's auto-updated :P I just emit always the event in the relation_changed [10:59] Chipaca: Is there a way to install python deps in the charm (similar to wheelhouse.txt) [10:59] ? [11:08] davigar15: There will be.. right now this process is a bit manual. We have one pattern described in the README in the repository [11:08] davigar15: We'll have a full building process and associated publication communication with the store [11:10] niemeyer: But that procedure is for adding deps from git repositories, right? [11:15] PR operator#242 closed: Stop test harness relation_ids raising KeyError [11:15] davigar15: Yes, but the lib directory may have any content on it, and it's added to sys.path [11:15] davigar15: Again, it's a bit manual.. we're working on the build tool [11:17] Muy buenos días a todos! [11:38] facubatista: Hey hey, good Friday [11:39] hola niemeyer, thanks! [11:54] brb, rebooting once again === mup__ is now known as mup_ [12:21] facubatista: 👋! [12:25] it's a bummer that 'juju ssh' doesn't populate JUJU_ things :-/ [12:25] Chipaca: It can't.. it doesn't have an agent behind it [12:25] Chipaca: That is, it's not in a hook context [12:26] Chipaca: That's what debug-hook / debug-code are for, and those do set it.. it's also done via ssh [12:28] niemeyer: debug-hook with no hooks, does that mean all, or none? [12:29] Chipaca: all [12:30] Chipaca: none wouldn't make much sense (debug nothing?) [12:31] juju noop [12:33] * Chipaca has a lot of fun pronouncing 'noop' as /n(j)uːp/ [12:47] I'd like to propose a breaking change: make the 'short' form observe look for _on_foo instead of of on_foo [12:47] (or, nuke the short form) [13:04] Chipaca: I vote for the latter, but we need a good way to notify people of the coming change [13:05] Chipaca: I'd also vote to do that at the same time that we promise API stability [13:05] Chipaca: We've done too many small breaking changes recently. They were good, but I think the time is approaching where we cannot afford to break any code anymore [13:06] niemeyer: I'd like to start having releases, too :) [13:10] Chipaca: Uhhh.. that's too advanced :P [13:19] * Chipaca giggles at 'juju run --operator' [13:19] davigar15: https://github.com/canonical/operator/issues/156 fwiw [13:21] mthaddon: BTW, what're you doing that makes it better to have a function do that instead of doing it at the package level? [13:22] Chipaca: sorry, not sure I understand the question - how do you mean? [13:23] I think stub and mthaddon understand that this is an ugly hack, but maybe we should add a note to the issue as well pointing that out [13:23] If you want to do something like that, do it on the install event, not at package level [13:23] mthaddon: I mean, https://git.launchpad.net/charm-k8s-wordpress/tree/src/wordpress.py#n14 [13:24] it means every single function starts with requests = import_requests() [13:24] Chipaca: ah, it was just that we were needing to do that in a few places so we broke it out into a functionn [13:25] Chipaca: yeah, we could have just done this at the package level too - either way felt less than ideal though [13:25] oh, agreed it's ugly, but doing it in a function is ugly _and_ awkward :-D [13:25] * Chipaca knows it's not like you have a nice option, here [13:26] I've added a note [13:26] Chipaca: There's a nice option in fact [13:26] Chipaca: There's an event called "install", which happens to be a good place to have "apt install" [13:27] niemeyer: did you see my request that you re-review #196? [13:27] PR #196: Factor out the Harness changes for test_model.py [13:27] Chipaca: I did.. I have not done it because jam is off and facundo is not. Have been working on something else instead, as we'll discuss in 3 minutes [13:28] niemeyer: "do it on install" does not fix having to make the name available outside of _on_install though [13:28] niemeyer: it also needs to be in a charm upgrade too, otherwise you might never call it again though I think? [13:28] at which point the approach in https://git.launchpad.net/charm-k8s-wordpress/tree/src/wordpress.py#n14 seems to be better [13:30] Chipaca: If the import and name is used inside functions, that's not a problem [13:31] Chipaca: WIll have a look after the standup [13:47] mthaddon: WRT reviewing that charm, can you create an issue in canonical/operator for it? including links to the code :) [13:47] sounds good, will do - thx [13:51] mthaddon: we're trying to get a process for this set up, i'll write it up sometime once we agree [13:52] Issue operator#243 opened: Review of wordpress kubernetes charm [13:52] thx, no hurry from our side, but any feedback appreciated [14:15] mthaddon: Are you available for a call just now? [14:15] niemeyer: yep [14:15] mthaddon: https://meet.google.com/fqw-mdqc-dsf [14:50] niemeyer: can you take a look at https://docs.google.com/document/d/1D-DA3vJRRaauNc2bAJqLTKtAMTTP6tdj5HmOz-rasJI/ and let me know if I've captured it? [14:51] facubatista: did you see my message about #233? [14:51] PR #233: Execute the registered methods to events under PDB if proper envvar is set [14:57] Chipaca, I didn't, but now I saw it [15:31] Chipaca, niemeyer, proposed now [15:32] * facubatista -> lunch [15:33] facubatista: nice [16:02] PR operator#244 opened: Draft of the ops library support [16:05] niemeyer: nice! [16:05] what a lovely note to wrap a week with :-) [16:05] Chipaca: Take it on! :) [16:05] those two PRs I mean [16:06] Chipaca: This still doesn't support the top-level 'opslib', but that's a nice future PR on top of the first merge [16:06] Chipaca: I did fix it to not import anything before it's actually used [16:06] niemeyer: i noticed [16:06] that's good [16:06] Chipaca: With the nice side effect that now import errors are reported normally [16:07] niemeyer: by 'take it on!' you mean I should grab the PR, make any changes I think it needs, and then drive it as if it were mine? [16:08] niemeyer: or just review as normal and then do follow-ups? [16:08] don't want to do the former if you mean the latter, and viceversa [16:13] Chipaca: Yeah, please feel free to just take ownership [16:13] ok, will do [16:13] Chipaca: I don't want to sit on this further and delay it [16:13] Chipaca: Thank you! [16:14] mthaddon: Doc looks great, thanks! [16:14] niemeyer: cool, thx for the suggestions - will incorporate those soon [16:17] niemeyer: that last bullet point for benefits is really nice (being able to break compatibility without breaking old code) [16:20] mthaddon: Yeah, we really need something that allows people to collaborate fast without clashing with or breaking each other [16:20] mthaddon: This looks painless enough I think [16:22] I'll try and find time to write up the charm developer workflow side of things next week as well, just so it's clear (to me at least) how that part works, but for now I'm EOW o/ [16:23] mthaddon: have a good one [16:23] i'm going to go see if my knee is up for a run [16:23] Chipaca: I'd check with both knees before you start :) [16:23] then, i'm going to break gustavo's work :-p [16:23] mthaddon: the one on the right votes conservative, it can do one [16:24] Or fix it, with tests! :) [16:24] Have fun folks === mup__ is now known as mup_ [17:04] * facubatista is back [19:10] I have a silly mistake in my branch, so I got an error on install, it goes to "error state", if it's broken, why the install is retried? [19:10] unit-bdv-4: 16:05:44 DEBUG unit.bdv/4.install AttributeError: module 'os' has no attribute 'chcwd' [19:10] unit-bdv-4: 16:05:45 ERROR juju.worker.uniter.operation hook "install" (via explicit, bespoke hook script) failed: exit status 1 [19:10] unit-bdv-4: 16:05:45 INFO juju.worker.uniter awaiting error resolution for "install" hook [19:10] unit-bdv-4: 16:05:50 INFO juju.worker.uniter awaiting error resolution for "install" hook [19:10] unit-bdv-4: 16:05:51 INFO unit.bdv/4.juju-log Installing blog... [19:10] failed: exit status 1 <-- [19:10] failed: exit status 1 <-- perfect! [19:11] juju.worker.uniter awaiting error resolution for "install" hook <-- perfect, will fix, let me get a mate [19:11] unit.bdv/4.juju-log Installing blog... <-- wait, why it started again?? [20:45] facubatista: yep, hooks that fail get retried by the juju [20:47] Chipaca, and I'm failing to update the code between retrials; and it doesn't let me to do a deploy again (it's says it's already deployed) [20:48] so the try/error cycle involves removing the app and deploying from scratch [20:48] so much lost time [20:49] facubatista: what happens if you ssh in and just cowboy the new charm in place? [20:49] You can ask to not repeat the hook [20:49] At resolve time [20:49] niemeyer: what's "resolve time" [20:50] niemeyer, I'm not running "resolve" [20:50] So how do you tell the unit to keep going? I must be out of context and if so sorry for that [20:51] niemeyer, I'm not telling it to continue, it retries by itself [20:51] I'd want to tell it "ok, please refresh the code and retry that install" [20:52] facubatista: There must be something broken then, or again I don't understand what's happening.. when a hook returns a non zero exit, juju puts it in an error state [20:55] niemeyer, it goes to error, yes, it even says "awaiting error resolution", but then retries out of the blue [20:55] Okay, I'm probably outdated then [20:56] This would not happen in the original implementation [21:17] * facubatista eods [21:17] and eows! [21:17] see you all on Monday, have a nice weekend [21:18] Have a good weekend [21:23] * Chipaca EOWs also [21:23] see y'all monday