| mup_ | PR operator#238 opened: ops/model.py: Unit.set_workload_version <Created by jameinel> <https://github.com/canonical/operator/pull/238> | 04:09 |
|---|---|---|
| mup_ | Issue operator#211 closed: Test harness: a unit receives events about updates to its own unit data bag <bug> <Created by dshcherb> <Closed by jameinel> <https://github.com/canonical/operator/issues/211> | 04:29 |
| jam | morning all | 04:46 |
| mup_ | Issue operator#239 opened: Action writing requires image dependencies <Created by natalytvinova> <https://github.com/canonical/operator/issues/239> | 07:03 |
| niemeyer | Morning jam | 07:50 |
| niemeyer | Hello all | 07:50 |
| jam | morning | 07:50 |
| Chipaca | moin moin | 09:07 |
| niemeyer | Hey Chipaca | 09:16 |
| jam | niemeyer, #196 still needs your ack, I believe both Chipaca and facubatista are happy with it | 10:38 |
| mup_ | PR #196: Factor out the Harness changes for test_model.py <Created by jameinel> <https://github.com/canonical/operator/pull/196> | 10:38 |
| jam | Chipaca, #236 is a small PR that hopefully isn't controversial, I needed to do *some* coding today. :) | 10:38 |
| mup_ | PR #236: ops/log.py: Default to DEBUG logging <Created by jameinel> <Merged by jameinel> <https://github.com/canonical/operator/pull/236> | 10:38 |
| jam | well, helps if I use the right PR: #238 | 10:39 |
| mup_ | PR #238: ops/model.py: Unit.set_workload_version <Created by jameinel> <https://github.com/canonical/operator/pull/238> | 10:39 |
| Chipaca | jam: huh, you can set something, but you can't get it? | 10:46 |
| Chipaca | that's super weird | 10:46 |
| jam | Chipaca, agreed, though it has been that way for about 5 years now, and it hasn't actually been raised as a bug by someone writing a charm | 10:46 |
| jam | Chipaca, as a way to tell the admin what is being run, the source of the value in that field is reading something else on the host machine, you might want to read it to see if it changed, but then again, you're just going to set it to the new value. | 10:57 |
| jam | which is as expensive as reading it | 10:57 |
| facubatista | Muy buenos días a todos! | 11:14 |
| jam | morning facubatista | 11:14 |
| facubatista | hola jam :) | 11:15 |
| Chipaca | facubatista: moin moin | 11:22 |
| facubatista | hola Chipaca | 11:23 |
| facubatista | Chipaca, I was wondering (and reading a little) if we can use GitHub Actions to store the latest commit (partial) hash in a file to be used as a reference in the version string, for "nightlies" or if people runs the project directly from repo | 11:28 |
| Chipaca | facubatista: what i've seen people do is have a distinctive "you're running this from an unreleased thing" version | 11:29 |
| facubatista | knowing which specific version is a plus | 11:30 |
| jam | facubatista, its pretty impossible to get the hash of the thing and commit it, because committing changes the hash.. | 11:32 |
| jam | it is usually a 'build' step (in a Makefile :) that generates the version.py from 'git describe --dirty' | 11:33 |
| facubatista | there's a [push] event that will trigger actions; the problem is that if we commit current hash on pushes, and then we rebase on merge to master, that previous commit (which is stored) is lost in the sense that we will not find it in the logs | 11:35 |
| facubatista | if we wouldn't rebase, that could be useful | 11:35 |
| jam | facubatista, you'll always be saying someone is using the version just-before the one they are actually using. | 11:36 |
| facubatista | jam, the only difference between the version they claim and the version they really using would be the version file change, though | 11:37 |
| jam | facubatista, if you turn every commit into 2 commits, I guess. | 11:37 |
| facubatista | but yes, the proper way to do this would be with support of git itself | 11:37 |
| facubatista | like "when commiting, after creating the hash, before saving everything, please dump that hash to this file" | 11:38 |
| facubatista | jam, reviewing your branch... you created set_workload_version which gets `version` and passes that directly to `self._backend.application_version_set`, which passes that directly to self._run, which passes that directly to subprocess.run | 11:52 |
| facubatista | jam, so if I do set_workload_version(5) it will explode super ugly | 11:52 |
| facubatista | maybe _run should str() the *args? | 11:52 |
| facubatista | (this is beyond your branch, of course, unless we decide "_run must always receive strings", so this would be something to change in your branch) | 11:53 |
| jam | facubatista, so the version should be a string, I don't know that silently auto casting the value to a string behind their back is the right way to do it | 11:55 |
| jam | I was actually going to put in a RuntimeError checking for it, but didn't feel it was significantly better than a TypeError | 11:55 |
| jam | (there is a test that TypeError is raised for non str) | 11:56 |
| jam | if you prefer a different error, I'm happy to do so | 11:56 |
| Chipaca | jam: maybe the docstring should talk about the type :) | 11:57 |
| facubatista | I'm ok with the TypeError, I'm even ok with the message itself, I'm scared about the traceback | 11:57 |
| facubatista | File "/usr/lib/python3.6/subprocess.py", line 1295, in _execute_child | 11:57 |
| facubatista | restore_signals, start_new_session, preexec_fn) | 11:57 |
| facubatista | TypeError: expected str, bytes or os.PathLike object, not int | 11:57 |
| Chipaca | jam: also, just noticed, "[...] show in the output of 'juju status'" should probably be 'shown' | 11:58 |
| facubatista | did we consider as a group about type hinting the exposed-to-the-developer interfaces? | 11:59 |
| jam | facubatista, Chipaca said that he had investigated some of the alternatives for how we do docstrings, etc. Part of that was whether we wanted to do type hints | 12:00 |
| jam | as it changes how we do docstrings if we're using type hints. | 12:00 |
| Chipaca | yep | 12:00 |
| Chipaca | i myself quite like them | 12:00 |
| Chipaca | but we're still to have that discussion | 12:01 |
| Chipaca | I'll add something in the calendar right now | 12:01 |
| jam | facubatista, does that mean we need top level error catching in the framework for nicer rendering? | 12:01 |
| facubatista | jam, maybe, it's a our-pain/user-friendliness balance | 12:01 |
| facubatista | (however, we *may* get that for free, after some automatic checking or something, if we have type hints in the "borders") | 12:03 |
| jam | Chipaca, changed from ", show" to "; shown" which I think is more correct. | 12:03 |
| jam | facubatista, the flip side is figuring out *who* is passing in an int, and that's hard to tell without the traceback | 12:03 |
| facubatista | jam, in my mind, the best user experience we can get here is that if you call set_workload_version with not a string, we should fail with something like TypeError("The workload version must be a string") | 12:04 |
| facubatista | jam, from that POV, what do you mean "who is passing an int"? | 12:04 |
| jam | facubatista, I'm happy to put that check into the method | 12:05 |
| Chipaca | facubatista, jam, it could do try/except TypeError as e: raise TypeError("The workload version...") from e | 12:05 |
| facubatista | jam, this is more a generic question than a specific request -- are we as a group considering this extra work for user friendliness? etc | 12:05 |
| Chipaca | facubatista: I don't think user friendliness should be thought of as 'extra work' :) | 12:06 |
| facubatista | Chipaca, well, we should check the rest of the public methods, then | 12:06 |
| jam | facubatista, Chipaca : updated the PR with a check. is it better to catch TypeError, or just do isinstance() ? | 12:09 |
| jam | facubatista, I do think giving users helpful guidance about how to use the interfaces is very much something we should do | 12:09 |
| facubatista | yeap, me too, that's why I brought the subject here | 12:10 |
| Chipaca | jam: I lean slightly towards the "try and catch" over the "check beforehand" camp, but not strongly | 12:10 |
| Chipaca | jam: but, maybe, in this case, checking is more what we need -- what happens if you try to set the version to "--help" ? | 12:11 |
| jam | Chipaca, 'juju status' says '--help' | 12:11 |
| jam | its the way you get SOS out | 12:11 |
| jam | you hack your charms to say "--help --meeeeeee" | 12:12 |
| Chipaca | jam: application-version-set won't try to print its help? | 12:12 |
| jam | Chipaca, interesting question. I'm not sure. but likely you could be correct | 12:13 |
| * Chipaca releases software with a version of --help all the time | 12:13 | |
| Chipaca | :-p | 12:13 |
| jam | Chipaca, I came across an interesting thing, where various websites like Banks have a secret link that lets you get domestic abuse help | 12:14 |
| jam | without "leaving the site" | 12:14 |
| Chipaca | interesting | 12:14 |
| facubatista | # application-version-set --help | 12:15 |
| facubatista | Usage: application-version-set <new-version> | 12:15 |
| facubatista | ... | 12:15 |
| Chipaca | facubatista: and 'application-version-set -- --help' ? | 12:15 |
| facubatista | Chipaca, worked | 12:16 |
| facubatista | App Version Status Scale Charm Store Rev OS Notes | 12:16 |
| facubatista | my-super-charm --help unknown 1 my-super-charm local 18 ubuntu | 12:16 |
| Chipaca | sounds like that's what we want to do | 12:17 |
| Chipaca | also, that's one for a "spread" suite (^ niemeyer) | 12:17 |
| Chipaca | as in, if we care about this (and i do), we'd want to have a test to make sure things don't break down the road (if juju changes options parser for ex) | 12:18 |
| Chipaca | lunch! | 12:20 |
| * Chipaca ⇝ lunch | 12:20 | |
| jam | Chipaca, so I'm happy to put in '--', but I'd caution around testing every possible interaction between many different components. | 12:20 |
| facubatista | maybe a change to include after we have the integration testing in place | 12:56 |
| jam | Chipaca, they fixed the 'tiled' layout | 13:05 |
| jam | It now does show everyone in little windows | 13:05 |
| jam | (at least in a group of 7, I see 6 windows) | 13:05 |
| facubatista | jam, wee | 13:07 |
| jam | and it was the default layout when joining | 13:07 |
| Chipaca | jam: i think they were rolling it out, because yesterday in the mgmt call some people had it and some didn't | 13:16 |
| Chipaca | sadly for me, i didn't :) | 13:16 |
| Chipaca | here's hoping! | 13:16 |
| Chipaca | jam: firefox? | 13:16 |
| Chipaca | hah, github just threw a 500 trying to show that pr | 13:21 |
| Chipaca | at least it's a cute 500 | 13:21 |
| jam | Chipaca, yeah, ff | 13:21 |
| Chipaca | ooh, it's all 500 all the way down | 13:22 |
| jam | Chipaca, are you sure it isn't you ? | 13:24 |
| Chipaca | jam: https://github.com/canonical/operator/pulls is 500ing, for ex | 13:25 |
| jam | your machine is now triggering 500 in *remote* machines | 13:25 |
| Chipaca | … and now it's back | 13:25 |
| jam | Chipaca, wfm | 13:25 |
| jam | :) | 13:25 |
| Chipaca | jam: I MUST NOT ABUSE THIS POWER | 13:25 |
| Chipaca | jam: given other people's grumbling elsewhere about this, it's not just me though | 13:27 |
| Chipaca | maybe it's all argentine-born people, because that's the one thing in common afaict | 13:27 |
| jam | Chipaca, yeah, other people in Juju were saying github is slow | 13:27 |
| * Chipaca hangs out in strange places | 13:28 | |
| niemeyer | I'm taking a break to go out into the sun for a while.. | 15:10 |
| mthaddon | hi folks, what's the right way to ask for review for a charm we've written? - not urgent in any way, would just like to get feedback from charmcraft team about what we've done and what could be done better | 15:26 |
| Chipaca | mthaddon: <3 | 15:27 |
| Chipaca | mthaddon: not sure, can i get back to you on that? | 15:27 |
| mthaddon | yep, no problem | 15:27 |
| Chipaca | mthaddon: is this charm public? | 15:27 |
| mthaddon | it is, yep | 15:27 |
| Chipaca | thinking of something code-review-ish | 15:28 |
| Chipaca | anyhoo, need to raise it with the team | 15:28 |
| mthaddon | https://jaas.ai/u/wordpress-charmers/wordpress and https://code.launchpad.net/charm-k8s-wordpress for the code | 15:28 |
| === jldev_ is now known as jldev | ||
| * Chipaca afk for a while | 16:34 | |
| * facubatista bb ~1h | 17:47 | |
| * facubatista is back | 18:51 | |
| * facubatista eods | 19:54 | |
| === mup__ is now known as mup_ | ||
| niemeyer | Have a good evening, Facundo | 19:55 |
| facubatista | niemeyer, thanks! see you tomorrow | 19:55 |
| niemeyer | o/ | 19:55 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!