| Chipaca | morning all | 07:48 |
|---|---|---|
| * Chipaca bbiab | 08:23 | |
| jam | morning | 08:28 |
| Chipaca | jam: how're you doing? | 08:36 |
| jam | pretty good, how about yourself? get any better rest? | 08:36 |
| Chipaca | yeah :-) overslept today but hey | 08:37 |
| Chipaca | hm, just realised i can make the logic a tiny bit simpler | 08:44 |
| * Chipaca goes for it | 08:44 | |
| Chipaca | hm, simpler but not clearer | 08:45 |
| * Chipaca undoes | 08:45 | |
| Chipaca | jam: i'm moving the bug revue to tomorrow | 09:23 |
| jam | ok | 09:23 |
| jam | you could move it to Friday :) | 09:23 |
| Chipaca | I could | 09:23 |
| Chipaca | but I won't | 09:23 |
| Chipaca | :-) | 09:23 |
| * Chipaca is also off Friday, but might have a masochist streak | 09:24 | |
| jam | Chipaca, 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 want | 09:54 |
| Chipaca | jam: 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 |
| Chipaca | so i think the current approach is the best we can do | 09:56 |
| jam | Chipaca, so I'm aware of 'timeit' I was wondering if there was anything better than that for Python benchmarks? | 09:57 |
| Chipaca | jam: I don't know of anything. I do feel like there has to be something :) | 09:58 |
| jam | pytest mentions benchmarking but there doesn't seem to be stdlib stuff. I do like go test's benchmark utility | 10:00 |
| jam | nose and pytest both seem to have something | 10:01 |
| jam | Chipaca, pytest doesn't like our test suite because you called TestMain "Test*" even though you didn't inherit from TestCase | 10:02 |
| jam | :) | 10:02 |
| Chipaca | jam: how so? | 10:16 |
| * Chipaca pokes | 10:16 | |
| Chipaca | ahh | 10:16 |
| jam | Chipaca, anything named "Test" gets run as a test, but it can't instantiate because it is an ABC | 10:16 |
| Chipaca | silly pytest | 10:16 |
| jam | https://gist.github.com/jameinel/c10f81ad708513eafa489a3cdb0ce046 | 10:16 |
| jam | Chipaca, ^^ seems like CSafeLoader is still important | 10:17 |
| jam | I grabbed a modest sized YAML file, and benchmarked loading it. 2ms vs 25ms | 10:17 |
| Chipaca | jam: 'modest' | 10:17 |
| Chipaca | :) | 10:17 |
| jam | Chipaca, it is postgresql's config.yaml: https://api.jujucharms.com/charmstore/v5/postgresql-207/archive/config.yaml | 10:17 |
| Chipaca | that aligns with what i remember when poking at snapcraft's usage of it too | 10:17 |
| jam | It isn't a book, but it isn't a trivial 2 line thing either. | 10:18 |
| Chipaca | ie 8-10x speedups | 10:18 |
| jam | 'modest' | 10:18 |
| jam | Chipaca, I'm actually rather surprised that PyYAML doesn't just use C if it is available | 10:18 |
| jam | having to have try/except /getattr in your code because they seem iffy about it seems odd | 10:18 |
| Chipaca | jam: you lost me there, what try/except/getattr? | 10:19 |
| Chipaca | jam: you mean to do Loader=<csafeloader if there otherwise safeloader>? | 10:19 |
| jam | Chipaca, 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 that | 10:20 |
| Chipaca | jam: agreed, that api is weird | 10:21 |
| Chipaca | jam: if you don't like try/except nor getattr, you could use yaml.__with_libyaml__ | 10:21 |
| Chipaca | jam: what did you use to build that benchmark btw? | 10:23 |
| jam | Chipaca, pip3 install --user pytest-benchmark and then wrote a single simple file and "pytest test_bench.py" | 10:24 |
| Chipaca | neat | 10:24 |
| jam | Chipaca, it has pretty output, so I figured its something reasonable to start with | 10:24 |
| jam | I'm still not a huge fan of pytest's magic variable names | 10:24 |
| jam | but their rendering is very nice | 10:24 |
| Chipaca | pretty output + super simple usage == win | 10:24 |
| Chipaca | pretty output + super simple usage - so much magic omg == … | 10:25 |
| Chipaca | i still can't decide that one :) | 10:25 |
| jam | Chipaca, 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 accident | 10:26 |
| jam | or a parameter that suddenly means something different because I changed a variable name | 10:26 |
| jam | If 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' implies | 10:27 |
| jam | Chipaca, so does that mean by default yaml.safe_load and yaml.load *don't* use the optimized backend? | 10:28 |
| Chipaca | jam: correct | 10:28 |
| jam | ugh | 10:28 |
| Chipaca | jam: load's default loader is FullLoader | 10:29 |
| Chipaca | jam: hopefully subclassing CSafeLoader to add tuples works :) | 10:31 |
| Chipaca | dunno | 10:31 |
| Chipaca | anyway. had a meeting, now some exercise before lunch and more meetings | 10:32 |
| jam | Chipaca, it passes the test suite :) | 10:33 |
| Chipaca | jam: huzzah :) | 10:33 |
| jam | Chipaca, 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 |
| jam | Chipaca, this feels very much in violation of the API rule "the obvious way should be the correct way" | 10:43 |
| Chipaca | jam: 'this is YAML. we don't do "obvious" here.' | 10:44 |
| Chipaca | ok, now yes i go | 10:45 |
| mup | PR operator#325 opened: ops/main.py: Use CSafeLoader <Created by jameinel> <https://github.com/canonical/operator/pull/325> | 10:47 |
| jam | Chipaca, ^^ when you're back | 10:47 |
| Chipaca | jam: i'm back | 10:53 |
| Chipaca | run has been postponed by rain :) | 10:54 |
| jam | I'm baaa-aaak | 10:56 |
| jam | https://youtu.be/QOkSvLqkafU?t=7 | 10:57 |
| * Chipaca concurs | 11:00 | |
| mup | PR operator#326 opened: make abc TestMain private to help pytest <Created by chipaca> <https://github.com/canonical/operator/pull/326> | 11:01 |
| jam | Chipaca, I'm available for standup whenever you are. why does YAML and python versions have to be so hard... | 13:25 |
| Chipaca | jam: let's do it | 13:28 |
| Chipaca | jam: so | 14:26 |
| Chipaca | jam: 1. make sure you have the appropriate python-dev on your system | 14:26 |
| Chipaca | jam: pip install cython | 14:26 |
| Chipaca | erm, that's (2) | 14:26 |
| Chipaca | jam: 3. make sure you nuke any previous yamls from the pip cache, find ~/.cache/pip/ -name 'PyYAML*' -delete | 14:27 |
| Chipaca | jam: 4. make sure you also have libyaml-dev etc (apt build-dep pyyaml or sth) | 14:27 |
| Chipaca | jam: now a pip install PyYAML should rebuild your wheel and get you the good stuff | 14:28 |
| jam | nuking pip cache is a bit of a pain for 3 different versions but at least we know | 14:28 |
| vgrevtsev | hi 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 process | 14:29 |
| Chipaca | jam: it only nukes the wheels, the source will still be there (and you want it rebuilt anyway) | 14:29 |
| vgrevtsev | so, that makes impossible to poll the application for the new status (which is relying on the pod spec params, ConfigMap for example) | 14:29 |
| vgrevtsev | is this an expected behaviour or should I raise a bug about that? | 14:29 |
| jam | Chipaca, $ pip3 uninstall yaml | 14:29 |
| jam | Traceback (most recent call last): | 14:29 |
| jam | ... | 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, sysconfig | 14:29 |
| jam | ImportError: 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 |
| jam | ( | 14:30 |
| * Chipaca covers his eyes and hides | 14:30 | |
| Chipaca | vgrevtsev: I think that's Juju's behaviour (we call pod-spec-set immediately) | 14:31 |
| vgrevtsev | :( | 14:31 |
| vgrevtsev | any ideas or suggestions how can we work around this in the charms? | 14:32 |
| jam | vgrevtsev, 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 discussion | 14:32 |
| vgrevtsev | jam: ah... alright. I will file a bug against Juju then. Thanks! | 14:32 |
| * vgrevtsev thinks what can he do to w/a this | 14:32 | |
| jam | vgrevtsev, how are you depending on a ConfigMap of another pod that you are creating, but then not setting ? | 14:33 |
| vgrevtsev | not another... | 14:33 |
| jam | Chipaca, fortunately I can just rm .venv and try again, but pip being broken from virtualenv is annoying | 14:33 |
| Chipaca | jam: I haven't seen that one myself, fwiw | 14:33 |
| jam | vgrevtsev, 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-set | 14:34 |
| jam | (note that I'm not saying you're wrong in any way, just trying to understand what is going on) | 14:35 |
| vgrevtsev | jam: 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 options | 14:35 |
| vgrevtsev | then, I tell my app via API to reload its config and trying to identify, has the config been changed or not | 14:35 |
| vgrevtsev | the problem here is, the config will never be in its new state, because the podspec has not been updated in reality | 14:36 |
| vgrevtsev | so, I can exit the config-changed handler (so the configmap will apply), but nobody then will tell an application to reload its state | 14:37 |
| jam | Chipaca, 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 |
| jam | vgrevtsev, 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 started | 14:40 |
| vgrevtsev | interesting thing, but somethimes it doesn't happen, for example if you're changing only the configmap part - it does not lead to pod re-creation | 14:42 |
| vgrevtsev | but if you'll change the `containerPort`, it will shut the pod down and create a new one, that's true | 14:42 |
| jam | vgrevtsev, so we had a discussion in the past about what should a charm do, and how does that act transactionally wrt other Juju model operations | 14:43 |
| jam | vgrevtsev, eg, 'relation-set' doesn't propagate values to other agents until the hook returns, and that is very much intentional | 14:44 |
| jam | pod-spec-set was seen a bit as something the charm does that then needs a similar transactionality | 14:44 |
| jam | however, I've come to see pod-spec-set as more like "systemd restart" | 14:44 |
| jam | which isn't under the same transaction as the hook | 14:44 |
| vgrevtsev | I'd be happy to have a way to forcibly restart/rebuild/... the pods. | 14:44 |
| vgrevtsev | That actually would solve my problem | 14:45 |
| vgrevtsev | Now I can see that pod-spec-set has been invoked, but the pod was left intact | 14:45 |
| vgrevtsev | so 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 |
| vgrevtsev | says* | 14:49 |
| Chipaca | spec_path.write_text(json.dumps(spec)) | 15:03 |
| Chipaca | i wonder why that's done that way around :) | 15:03 |
| jam | Chipaca, vs json.dump(open(spec_path)) ? | 15:04 |
| Chipaca | jam: or with spec_path.open(...) … | 15:04 |
| Chipaca | yeh | 15:04 |
| vgrevtsev | jam: fwiw, https://bugs.launchpad.net/juju/+bug/1882976 | 15:51 |
| Chipaca | vgrevtsev: 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 |
| vgrevtsev | was thinking about putting the reload logic in the update-status hook btw | 16:17 |
| Chipaca | anyhoo, soft-eod from me, will check in later | 16:19 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!