/srv/irclogs.ubuntu.com/2020/04/24/#smooth-operator.txt

Chipacamo'in!08:04
niemeyerMorning Chipaca, greetings all08:09
Chipacalaptop is once again stuck at 800MHz, I'm going to have to take it apart and unplug the battery again :-|08:13
Chipacaprobably on the weekend08:13
Chipacaso, 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 var08:42
Chipacato 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
Chipacathis does not seem too bad to me08:43
niemeyerChipaca: 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 env08:44
Chipacaniemeyer: 'the former approach', you mean looking at argv when we're _not_ dispatch?08:44
Chipacathat seems fine08:44
niemeyerChipaca: Yeah, what we have atm08:44
Chipacaniemeyer: 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:46
niemeyerChipaca: We can register something related and still keep the Python module name08:54
niemeyerChipaca: We should circle back to see how to press further on obsolescence of the name, though08:55
niemeyerChipaca: It's certainly abandoned for long08:55
Chipacaniemeyer: pypi has a definition of abandoned that is not just 'has it seen a release'08:56
Chipacaworking on it, in any case08:56
niemeyerChipaca: Sure, but that seems like a valid case of that for all metrics I can imagine08:56
mup_Issue operator#240 opened: Shared StoredState between kubernetes workload pods <Created by davigar15> <https://github.com/canonical/operator/issues/240>09:27
jam niemeyer, Chipaca : maybe they use the metric "has it been installed lately" and most of us have accidentally installed it recently :)09:27
Chipacathat is one of the things they look at, yes09:33
davigar15Chipaca: Dmitrii-Sh and I have found this bug: https://github.com/canonical/operator/issues/24009:35
* Chipaca looks09:36
Chipacayouch09:37
Chipacadavigar15: out of ignorance, what creates the symlink from agents/<unit>/charm to agents/<app>/charm ?09:39
Chipacais that juju itself?09:39
Chipacaniemeyer, jam, ^^09:39
jamChipaca, most likely Juju, sounds like on K8s every unit shares the charm dir, which we didn't expect09:40
Chipacajam: isn't it fairly common to use the charm dir to store stuff, like keys and the like?09:41
jamChipaca, 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 uses09:42
jamused09:42
Chipacajam: so we're doing something we know we shouldn't?09:43
jamChipaca, 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:44
jamChipaca, 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_dir09:45
Chipacajam: yeah, i was about to ask, how _can_ a charm grab a directory for its own use?09:45
jamChipaca, 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 recommendation09:46
jamit 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:46
mup_Issue operator#241 opened: _TestingModelBackend incorrectly raises KeyError on missing relation <Created by gnuoy> <https://github.com/canonical/operator/issues/241>09:50
mup_PR operator#242 opened: Stop test harness relation_ids raising KeyError <Created by gnuoy> <https://github.com/canonical/operator/pull/242>09:52
jamanyway, not really here, just passing by, heading back to other activities10:02
jamChipaca, #241 should be fixed by #196, IIRC10:02
mup_Issue #241: _TestingModelBackend incorrectly raises KeyError on missing relation <Created by gnuoy> <https://github.com/canonical/operator/issues/241>10:02
mup_PR #196: Factor out the Harness changes for test_model.py <Created by jameinel> <https://github.com/canonical/operator/pull/196>10:02
Chipacaniemeyer: could you re-review #196 ? i think jam addressed your points10:19
mup_PR #196: Factor out the Harness changes for test_model.py <Created by jameinel> <https://github.com/canonical/operator/pull/196>10:19
Chipacadavigar15: is that bug blockig your work?10:23
davigar15Chipaca: No, I have a workaround for it :-)10:25
Chipacadavigar15: ok10:25
Chipacathen 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 bbiab10:25
davigar15Basically 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_changed10:26
davigar15Chipaca: Is there a way to install python deps in the charm (similar to wheelhouse.txt)10:59
davigar15?10:59
niemeyerdavigar15: There will be.. right now this process is a bit manual. We have one pattern described in the README in the repository11:08
niemeyerdavigar15: We'll have a full building process and associated publication communication with the store11:08
davigar15niemeyer: But that procedure is for adding deps from git repositories, right?11:10
mup_PR operator#242 closed: Stop test harness relation_ids raising KeyError <Created by gnuoy> <Closed by gnuoy> <https://github.com/canonical/operator/pull/242>11:15
niemeyerdavigar15: Yes, but the lib directory may have any content on it, and it's added to sys.path11:15
niemeyerdavigar15: Again, it's a bit manual.. we're working on the build tool11:15
facubatistaMuy buenos días a todos!11:17
niemeyerfacubatista: Hey hey, good Friday11:38
facubatistahola niemeyer, thanks!11:39
Chipacabrb, rebooting once again11:54
=== mup__ is now known as mup_
Chipacafacubatista: 👋!12:21
Chipacait's a bummer that 'juju ssh' doesn't populate JUJU_ things :-/12:25
niemeyerChipaca: It can't.. it doesn't have an agent behind it12:25
niemeyerChipaca: That is, it's not in a hook context12:25
niemeyerChipaca: That's what debug-hook / debug-code are for, and those do set it.. it's also done via ssh12:26
Chipacaniemeyer: debug-hook with no hooks, does that mean all, or none?12:28
niemeyerChipaca: all12:29
niemeyerChipaca: none wouldn't make much sense (debug nothing?)12:30
facubatistajuju noop12:31
* Chipaca has a lot of fun pronouncing 'noop' as /n(j)uːp/12:33
ChipacaI'd like to propose a breaking change: make the 'short' form observe look for _on_foo instead of of on_foo12:47
Chipaca(or, nuke the short form)12:47
niemeyerChipaca: I vote for the latter, but we need a good way to notify people of the coming change13:04
niemeyerChipaca: I'd also vote to do that at the same time that we promise API stability13:05
niemeyerChipaca: 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 anymore13:05
Chipacaniemeyer: I'd like to start having releases, too :)13:06
niemeyerChipaca: Uhhh.. that's too advanced :P13:10
* Chipaca giggles at 'juju run --operator'13:19
mthaddondavigar15: https://github.com/canonical/operator/issues/156 fwiw13:19
Chipacamthaddon: BTW, what're you doing that makes it better to have a function do that instead of doing it at the package level?13:21
mthaddonChipaca: sorry, not sure I understand the question - how do you mean?13:22
niemeyerI 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 out13:23
niemeyerIf you want to do something like that, do it on the install event, not at package level13:23
Chipacamthaddon: I mean, https://git.launchpad.net/charm-k8s-wordpress/tree/src/wordpress.py#n1413:23
Chipacait means every single function starts with requests = import_requests()13:24
mthaddonChipaca: ah, it was just that we were needing to do that in a few places so we broke it out into a functionn13:24
mthaddonChipaca: yeah, we could have just done this at the package level too - either way felt less than ideal though13:25
Chipacaoh, agreed it's ugly, but doing it in a function is ugly _and_ awkward :-D13:25
* Chipaca knows it's not like you have a nice option, here13:25
niemeyerI've added a note13:26
niemeyerChipaca: There's a nice option in fact13:26
niemeyerChipaca: There's an event called "install", which happens to be a good place to have "apt install"13:26
Chipacaniemeyer: did you see my request that you re-review #196?13:27
mup_PR #196: Factor out the Harness changes for test_model.py <Created by jameinel> <https://github.com/canonical/operator/pull/196>13:27
niemeyerChipaca: 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 minutes13:27
Chipacaniemeyer: "do it on install" does not fix having to make the name available outside of _on_install though13:28
mthaddonniemeyer: it also needs to be in a charm upgrade too, otherwise you might never call it again though I think?13:28
Chipacaat which point the approach in https://git.launchpad.net/charm-k8s-wordpress/tree/src/wordpress.py#n14 seems to be better13:28
niemeyerChipaca: If the import and name is used inside functions, that's not a problem13:30
niemeyerChipaca: WIll have a look after the standup13:31
Chipacamthaddon: WRT reviewing that charm, can you create an issue in canonical/operator for it? including links to the code :)13:47
mthaddonsounds good, will do - thx13:47
Chipacamthaddon: we're trying to get a process for this set up, i'll write it up sometime once we agree13:51
mup_Issue operator#243 opened: Review of wordpress kubernetes charm <Created by mthaddon> <https://github.com/canonical/operator/issues/243>13:52
mthaddonthx, no hurry from our side, but any feedback appreciated13:52
niemeyermthaddon: Are you available for a call just now?14:15
mthaddonniemeyer: yep14:15
niemeyermthaddon: https://meet.google.com/fqw-mdqc-dsf14:15
mthaddonniemeyer: 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:50
Chipacafacubatista: did you see my message about #233?14:51
mup_PR #233: Execute the registered methods to events under PDB if proper envvar is set <Created by facundobatista> <https://github.com/canonical/operator/pull/233>14:51
facubatistaChipaca, I didn't, but now I saw it14:57
facubatistaChipaca, niemeyer, proposed now15:31
* facubatista -> lunch15:32
Chipacafacubatista: nice15:33
mup_PR operator#244 opened: Draft of the ops library support <Created by niemeyer> <https://github.com/canonical/operator/pull/244>16:02
Chipacaniemeyer: nice!16:05
Chipacawhat a lovely note to wrap a week with :-)16:05
niemeyerChipaca: Take it on! :)16:05
Chipacathose two PRs I mean16:05
niemeyerChipaca: This still doesn't support the top-level 'opslib', but that's a nice future PR on top of the first merge16:06
niemeyerChipaca: I did fix it to not import anything before it's actually used16:06
Chipacaniemeyer: i noticed16:06
Chipacathat's good16:06
niemeyerChipaca: With the nice side effect that now import errors are reported normally16:06
Chipacaniemeyer: 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:07
Chipacaniemeyer: or just review as normal and then do follow-ups?16:08
Chipacadon't want to do the former if you mean the latter, and viceversa16:08
niemeyerChipaca: Yeah, please feel free to just take ownership16:13
Chipacaok, will do16:13
niemeyerChipaca: I don't want to sit on this further and delay it16:13
niemeyerChipaca: Thank you!16:13
niemeyermthaddon: Doc looks great, thanks!16:14
mthaddonniemeyer: cool, thx for the suggestions - will incorporate those soon16:14
mthaddonniemeyer: that last bullet point for benefits is really nice (being able to break compatibility without breaking old code)16:17
niemeyermthaddon: Yeah, we really need something that allows people to collaborate fast without clashing with or breaking each other16:20
niemeyermthaddon: This looks painless enough I think16:20
mthaddonI'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:22
Chipacamthaddon: have a good one16:23
Chipacai'm going to go see if my knee is up for a run16:23
mthaddonChipaca: I'd check with both knees before you start :)16:23
Chipacathen, i'm going to break gustavo's work :-p16:23
Chipacamthaddon: the one on the right votes conservative, it can do one16:23
niemeyerOr fix it, with tests! :)16:24
niemeyerHave fun folks16:24
=== mup__ is now known as mup_
* facubatista is back17:04
facubatistaI 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
facubatistaunit-bdv-4: 16:05:44 DEBUG unit.bdv/4.install AttributeError: module 'os' has no attribute 'chcwd'19:10
facubatistaunit-bdv-4: 16:05:45 ERROR juju.worker.uniter.operation hook "install" (via explicit, bespoke hook script) failed: exit status 119:10
facubatistaunit-bdv-4: 16:05:45 INFO juju.worker.uniter awaiting error resolution for "install" hook19:10
facubatistaunit-bdv-4: 16:05:50 INFO juju.worker.uniter awaiting error resolution for "install" hook19:10
facubatistaunit-bdv-4: 16:05:51 INFO unit.bdv/4.juju-log Installing blog...19:10
facubatistafailed: exit status 1 <--19:10
facubatistafailed: exit status 1 <-- perfect!19:10
facubatistajuju.worker.uniter awaiting error resolution for "install" hook <-- perfect, will fix, let me get a mate19:11
facubatistaunit.bdv/4.juju-log Installing blog... <-- wait, why it started again??19:11
Chipacafacubatista: yep, hooks that fail get retried by the juju20:45
facubatistaChipaca, 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:47
facubatistaso the try/error cycle involves removing the app and deploying from scratch20:48
facubatistaso much lost time20:48
Chipacafacubatista: what happens if you ssh in and just cowboy the new charm in place?20:49
niemeyerYou can ask to not repeat the hook20:49
niemeyerAt resolve time20:49
Chipacaniemeyer: what's "resolve time"20:49
facubatistaniemeyer, I'm not running "resolve"20:50
niemeyerSo how do you tell the unit to keep going? I must be out of context and if so sorry for that20:50
facubatistaniemeyer, I'm not telling it to continue, it retries by itself20:51
facubatistaI'd want to tell it "ok, please refresh the code and retry that install"20:51
niemeyerfacubatista: 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 state20:52
facubatistaniemeyer, it goes to error, yes, it even says "awaiting error resolution", but then retries out of the blue20:55
niemeyerOkay, I'm probably outdated then20:55
niemeyerThis would not happen in the original implementation20:56
* facubatista eods21:17
facubatistaand eows!21:17
facubatistasee you all on Monday, have a nice weekend21:17
niemeyerHave a good weekend21:18
* Chipaca EOWs also21:23
Chipacasee y'all monday21:23

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