Chipacamorning all07:48
* Chipaca bbiab08:23
Chipacajam: how're you doing?08:36
jampretty good, how about yourself? get any better rest?08:36
Chipacayeah :-) overslept today but hey08:37
Chipacahm, just realised i can make the logic a tiny bit simpler08:44
* Chipaca goes for it08:44
Chipacahm, simpler but not clearer08:45
* Chipaca undoes08:45
Chipacajam: i'm moving the bug revue to tomorrow09:23
jamyou could move it to Friday :)09:23
ChipacaI could09:23
Chipacabut I won't09:23
* Chipaca is also off Friday, but might have a masochist streak09:24
jamChipaca, is there any explicit Juju testing that you would like me to do? I'm comfortable with your patch, but I can do a version test if you want09:54
Chipacajam: i'm happy with it too. If we had concrete steps to reproduce an issue with low 2.7.x then I might ask to look at them, but we don't...09:56
Chipacaso i think the current approach is the best we can do09:56
jamChipaca, so I'm aware of 'timeit' I was wondering if there was anything better than that for Python benchmarks?09:57
Chipacajam: I don't know of anything. I do feel like there has to be something :)09:58
jampytest mentions benchmarking but there doesn't seem to be stdlib stuff. I do like go test's benchmark utility10:00
jamnose and pytest both seem to have something10:01
jamChipaca, pytest doesn't like our test suite because you called TestMain "Test*" even though you didn't inherit from TestCase10:02
Chipacajam: how so?10:16
* Chipaca pokes10:16
jamChipaca, anything named "Test" gets run as a test, but it can't instantiate because it is an ABC10:16
Chipacasilly pytest10:16
jamChipaca, ^^ seems like CSafeLoader is still important10:17
jamI grabbed a modest sized YAML file, and benchmarked loading it. 2ms vs 25ms10:17
Chipacajam: 'modest'10:17
jamChipaca, it is postgresql's config.yaml: https://api.jujucharms.com/charmstore/v5/postgresql-207/archive/config.yaml10:17
Chipacathat aligns with what i remember when poking at snapcraft's usage of it too10:17
jamIt isn't a book, but it isn't a trivial 2 line thing either.10:18
Chipacaie 8-10x speedups10:18
jamChipaca, I'm actually rather surprised that PyYAML doesn't just use C if it is available10:18
jamhaving to have try/except /getattr in your code because they seem iffy about it seems odd10:18
Chipacajam: you lost me there, what try/except/getattr?10:19
Chipacajam: you mean to do Loader=<csafeloader if there otherwise safeloader>?10:19
jamChipaca, yeah. Having the API to your library be "wrap everything in a check to see if you actually have what you want" rather than providing that10:20
Chipacajam: agreed, that api is weird10:21
Chipacajam: if you don't like try/except nor getattr, you could use yaml.__with_libyaml__10:21
Chipacajam: what did you use to build that benchmark btw?10:23
jamChipaca, pip3 install --user pytest-benchmark and then wrote a single simple file and "pytest test_bench.py"10:24
jamChipaca, it has pretty output, so I figured its something reasonable to start with10:24
jamI'm still not a huge fan of pytest's magic variable names10:24
jambut their rendering is very nice10:24
Chipacapretty output + super simple usage == win10:24
Chipacapretty output + super simple usage - so much magic omg == …10:25
Chipacai still can't decide that one :)10:25
jamChipaca, it goes back to discovery/surprise. If I write a function and it magically starts testing it, 'wow'. But then I have TestMain that gets run by accident10:26
jamor a parameter that suddenly means something different because I changed a variable name10:26
jamIf you're deep in pytest it is probably reasonable and avoids boilerplate. When you're not, then it is inscrutable and you don't know what 'caplog' implies10:27
jamChipaca, so does that mean by default yaml.safe_load and yaml.load *don't* use the optimized backend?10:28
Chipacajam: correct10:28
Chipacajam: load's default loader is FullLoader10:29
Chipacajam: hopefully subclassing CSafeLoader to add tuples works :)10:31
Chipacaanyway. had a meeting, now some exercise before lunch and more meetings10:32
jamChipaca, it passes the test suite :)10:33
Chipacajam: huzzah :)10:33
jamChipaca, interestingly, that is a 20ms speedup per hook for Postgresql, since we load metadata.yaml on every hook (at least it isn't every call to Juju, but its still a fair bit)10:42
jamChipaca, this feels very much in violation of the API rule "the obvious way should be the correct way"10:43
Chipacajam: 'this is YAML. we don't do "obvious" here.'10:44
Chipacaok, now yes i go10:45
mupPR operator#325 opened: ops/main.py: Use CSafeLoader <Created by jameinel> <https://github.com/canonical/operator/pull/325>10:47
jamChipaca, ^^ when you're back10:47
Chipacajam: i'm back10:53
Chipacarun has been postponed by rain :)10:54
jamI'm baaa-aaak10:56
* Chipaca concurs11:00
mupPR operator#326 opened: make abc TestMain private to help pytest <Created by chipaca> <https://github.com/canonical/operator/pull/326>11:01
jamChipaca, I'm available for standup whenever you are. why does YAML and python versions have to be so hard...13:25
Chipacajam: let's do it13:28
Chipacajam: so14:26
Chipacajam: 1. make sure you have the appropriate python-dev on your system14:26
Chipacajam: pip install cython14:26
Chipacaerm, that's (2)14:26
Chipacajam: 3. make sure you nuke any previous yamls from the pip cache, find ~/.cache/pip/ -name 'PyYAML*' -delete14:27
Chipacajam: 4. make sure you also have libyaml-dev etc (apt build-dep pyyaml or sth)14:27
Chipacajam: now a pip install PyYAML should rebuild your wheel and get you the good stuff14:28
jamnuking pip cache is a bit of a pain for 3 different versions but at least we know14:28
vgrevtsevhi everyone - still working on the Prom charm and found something that I'd like to discuss... so tl;dr when I'm doing a `model.pod.set_spec` inside of the hook (let's say, `config-changed`) - it applies  ONLY after the config-changed hook will exit, not in the middle of the process14:29
Chipacajam: it only nukes the wheels, the source will still be there (and you want it rebuilt anyway)14:29
vgrevtsevso, that makes impossible to poll the application for the new status (which is relying on the pod spec params, ConfigMap for example)14:29
vgrevtsevis this an expected behaviour or should I raise a bug about that?14:29
jamChipaca, $ pip3 uninstall yaml14:29
jamTraceback (most recent call last):14:29
jam  File "/home/jameinel/dev/operator/.venv/3.8/lib/python3.8/distutils/__init__.py", line 1, in <module>14:29
jam    from distutils import dist, sysconfig14:29
jamImportError: cannot import name 'dist' from partially initialized module 'distutils' (most likely due to a circular import) (/home/jameinel/dev/operator/.venv/3.8/lib/python3.8/distutils/__init__.py)14:30
* Chipaca covers his eyes and hides14:30
Chipacavgrevtsev: I think that's Juju's behaviour (we call pod-spec-set immediately)14:31
vgrevtsevany ideas or suggestions how can we work around this in the charms?14:32
jamvgrevtsev, I do know that it is intentional behavior. You're welcome to file a bug about it, I have discussed it a bit, but it is good to know why you need it and lend your voice to the discussion14:32
vgrevtsevjam: ah... alright. I will file a bug against Juju then. Thanks!14:32
* vgrevtsev thinks what can he do to w/a this14:32
jamvgrevtsev, how are you depending on a ConfigMap of another pod that you are creating, but then not setting ?14:33
vgrevtsevnot another...14:33
jamChipaca, fortunately I can just rm .venv and try again, but pip being broken from virtualenv is annoying14:33
Chipacajam: I haven't seen that one myself, fwiw14:33
jamvgrevtsev, so, you're running the charm in the -operator pod, you are calling pod-spec-set to create a different pod (for the application), and then what config map entry exists for that pod that you didn't just set from pod-spec-set14:34
jam(note that I'm not saying you're wrong in any way, just trying to understand what is going on)14:35
vgrevtsevjam: so my usecase is - I'm deploying the pod which has some ConfigMap, and then I'd like to re-configure this pod (that's what `config-changed` for, right?) So I'm setting the new PodSpec against my application pod with the new configmap options14:35
vgrevtsevthen, I tell my app via API to reload its config and trying to identify, has the config been changed or not14:35
vgrevtsevthe problem here is, the config will never be in its new state, because the podspec has not been updated in reality14:36
vgrevtsevso, I can exit the config-changed handler (so the configmap will apply), but nobody then will tell an application to reload its state14:37
jamChipaca, worked for 3.6 and 3.7, for 3.8 I get warnings about "char*" vs "unsigned char*" but I *think* it worked...14:40
jamvgrevtsev, if you actually changed the pod spec, then Juju will tell K8s to schedule the new schema, causing your pod to get stopped and a new one started14:40
vgrevtsevinteresting thing, but somethimes it doesn't happen, for example if you're changing only the configmap part - it does not lead to pod re-creation14:42
vgrevtsevbut if you'll change the `containerPort`, it will shut the pod down and create a new one, that's true14:42
jamvgrevtsev, so we had a discussion in the past about what should a charm do, and how does that act transactionally wrt other Juju model operations14:43
jamvgrevtsev, eg, 'relation-set' doesn't propagate values to other agents until the hook returns, and that is very much intentional14:44
jampod-spec-set was seen a bit as something the charm does that then needs a similar transactionality14:44
jamhowever, I've come to see pod-spec-set as more like "systemd restart"14:44
jamwhich isn't under the same transaction as the hook14:44
vgrevtsevI'd be happy to have a way to forcibly restart/rebuild/... the pods.14:44
vgrevtsevThat actually would solve my problem14:45
vgrevtsevNow I can see that pod-spec-set has been invoked, but the pod was left intact14:45
vgrevtsevso that leaves an application in non-consistent state, as charm config says `foo=baz`, while app still thinks `foo=bar`, for example :)14:47
vgrevtsev(while k8s's ConfigMap stays `foo=baz`, as it got updated by pod-spec-set)14:47
Chipaca            spec_path.write_text(json.dumps(spec))15:03
Chipacai wonder why that's done that way around :)15:03
jamChipaca, vs json.dump(open(spec_path)) ?15:04
Chipacajam: or with spec_path.open(...) …15:04
vgrevtsevjam: fwiw, https://bugs.launchpad.net/juju/+bug/188297615:51
Chipacavgrevtsev: as a (probably sucky) workaround, you could defer and then when you come back the change should be applied (but it'll be 5 minutes later)16:17
vgrevtsevwas thinking about putting the reload logic in the update-status hook btw16:17
Chipacaanyhoo, soft-eod from me, will check in later16:19

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