mthaddon | jam: you may be interested in https://code.launchpad.net/~pjdc/charm-k8s-mattermost/+git/charm-k8s-mattermost/+merge/385953 - just looking over it now myself | 07:00 |
---|---|---|
jam | mthaddon, thanks for the heads up | 07:22 |
mthaddon | jam: have just asked for some updates - I noticed the test pod spec structure is a bit different from what we're actually defining in the charm | 07:25 |
Chipaca | 👋 | 08:46 |
jam | heh | 09:16 |
jam | so it actually was something different than I thought | 09:17 |
jam | Chipaca, https://paste.ubuntu.com/p/4RKzm4PW5h/ | 09:17 |
jam | so I noticed that if I did: foo: "bar: 42" | state-set --file - | 09:17 |
jam | then when I did 'state-get' | 09:17 |
jam | It would say: | 09:17 |
jam | foo: "bar: 42" | 09:17 |
jam | but if I did | 09:17 |
jam | foo: |\n bar: 42 | state-set -file - | 09:18 |
jam | then the output would be | 09:18 |
jam | foo: | | 09:18 |
jam | bar: 42 | 09:18 |
jam | However, that is because those 2 strings are subtly different, can you tell why? | 09:18 |
Chipaca | jam: newline? | 09:19 |
jam | Chipaca, yep '|' includes a trailing newline '|-' and the one-line form do not | 09:19 |
Chipaca | phew | 09:19 |
Chipaca | if it wasn't that, it was cursed :-) | 09:20 |
jam | Chipaca, you can see the output of 'state-get foo' was 'bar: 42\n\n' | 09:20 |
jam | I thought it was somehow remembering how I set the value and preserving it, which didn't really make sense | 09:20 |
Chipaca | bar: 12\n\n i guess? | 09:20 |
Chipaca | i believe preserving layout was one of the goals of gustavo's yaml.v3, but i'm not 100% positive on that (nor on whether he achieved it) | 09:21 |
Chipaca | i never got to play with that, actually | 09:21 |
Chipaca | still on my ToDo | 09:21 |
jam | Chipaca, that is as go not python, though, right? | 09:23 |
jam | and not a YAML spec but just a parser quirk? | 09:23 |
Chipaca | correct | 09:23 |
jam | Chipaca, do we think it is a problem if we treat foo='' as NoSnapshot ? | 09:24 |
Chipaca | and maybe it wasn't layout but comments? | 09:24 |
jam | It is fine as long as your snapshot isn't just a string that can be empty | 09:24 |
jam | (we could enforce snapshots are dicts again :) | 09:24 |
jam | Chipaca, or I could do the "if we get '' read the whole state-get and see whether the key exists" | 09:24 |
Chipaca | so this is only a problem for a snapshot that serialises as a string, and only if it cares about the difference between a string and not being there | 09:25 |
Chipaca | right? | 09:25 |
Chipaca | snapshot returns a simple serialisable object and the ones writing it out into yaml is us, right? | 09:26 |
Chipaca | so what we could do, if the above is right, is !!str strings | 09:26 |
jam | Chipaca, actually, it isn't a problem, because we yaml.dump('') => "''\n" | 09:26 |
Chipaca | and then we know because we get '' vs '!!str ""' | 09:26 |
jam | yaml.load('') => None, but yaml.lod("''\n") => '' | 09:26 |
Chipaca | ahhhh | 09:26 |
Chipaca | also aaaaah *and* ahahaha | 09:27 |
Chipaca | jam: cool cool, then :) | 09:27 |
jam | Chipaca, I'll make sure to add '' to my permutation tests | 09:27 |
jam | hm. seems I also need to handle None | 09:31 |
jam | >>> print(yaml.dump(None)) | 09:31 |
jam | null | 09:31 |
jam | ... | 09:31 |
jam | yaml.safe_load('') => None, yaml.safe_load('null\n...\n') => None | 09:32 |
jam | Chipaca, and, of course, I'm simulating some of this via my own scripts, which is definitely stuff that I'd want to test against actual Juju | 09:35 |
Chipaca | jam: do we have a list of 'things we want to try in real jujus tee emm'? (should we?) | 09:43 |
jam | Chipaca, well, I consider that writing a 'juju test suite' as part of test-main was one of my goals of this PR | 09:44 |
Chipaca | facundo__: 'charmcraft --verbose build' not being exactly the same as 'charmcraft build --verbose' is surprising and painful | 09:45 |
jam | but we can split it out if we prefer | 09:45 |
Chipaca | jam: i hadn't realised that, is there any of this already up? (how were you approaching it?) | 09:46 |
Chipaca | s/were/are/ | 09:46 |
jam | Chipaca, that was part of my trying to make test-main be a RealCharm | 09:48 |
jam | but it isn't something I've actively started | 09:48 |
jam | Chipaca, my intent was to add an action to test_main, such that you could run "juju deploy test_main; juju run-action ops-tests" | 09:48 |
Chipaca | jam: given you can make travis install snaps, and in particular lxd snaps, i was hoping we'd be able to use that to run tests | 09:48 |
jam | Chipaca, snap install lxd; snap install --classic juju; juju bootstrap lxd; juju deploy; juju run-action | 09:49 |
jam | Chipaca, I don't expect it to be very fast (bootstrap lxd here is a minute or so) | 09:49 |
Chipaca | jam: i was thinking of looking at having it as a cron test rather than unit tests unless we got them to be faster than i expect them to be, though | 09:49 |
Chipaca | yeah | 09:50 |
Chipaca | i mean, up to ~5 minutes is probably fine for PR tests | 09:50 |
Chipaca | longer than that and i'd punt them to cron / manual | 09:50 |
Chipaca | heh, https://github.com/juju-solutions/charms.reactive/blob/master/.travis.yml | 09:54 |
Chipaca | and a job run: https://travis-ci.org/github/juju-solutions/charms.reactive/jobs/675789700 | 09:55 |
Chipaca | (somebody needs to tweak that travis, but it essentially seems to work?) | 09:55 |
Chipaca | anyhoo | 09:55 |
jam | Chipaca, "ran for 2min43s" isn't bad | 09:58 |
jam | apt install snapd pwgen is 80s of it | 09:59 |
Chipaca | jam: "didn't actually *run* run" kinda is tho =) | 09:59 |
jam | yeah, I see that now | 09:59 |
Chipaca | i mean, juju bootstrap fell over | 09:59 |
jam | It is doing the usermod, but not doing the newgrp so the active process knows it is in the group | 09:59 |
Chipaca | hopefully it's that and not some weird apparmor-inside-apparmor-inside-apparmor thing | 10:00 |
Chipaca | (travis already runs inside lxd in some situations) | 10:00 |
Chipaca | (i think it's when you're doing weird arches, and then it's in vms and not containers, but still) | 10:00 |
jam | Chipaca, ah right, they recently switched so amd64 can be run in a container | 10:05 |
jam | Chipaca, the actual failure is that it can't talk to the lxd socket, but that might be because lxd failed to start? | 10:06 |
jam | Chipaca, do you have any quick magic for timing what is taking long in a test run? | 10:06 |
jam | I've gotten some stuff to work with cProfile, etc, but it is always a bit clumsier than I would expect | 10:07 |
Chipaca | jam: i've monkeypatched the test constructors to track time | 10:07 |
Chipaca | jam: otoh pytest i think has things for that? | 10:07 |
Chipaca | let me see if i still have the monkey patching thing somewhere | 10:08 |
Chipaca | jam: pytest --durations=10 will print the 10 slowest tests | 10:09 |
jam | Chipaca, yeah, pytest-profiling sems to get me where I'd like to be | 10:09 |
jam | py.test test/test_storage.py::TestJujuStorage::test_emit_event --profile | 10:12 |
jam | gives me a cumulative time spent | 10:12 |
Chipaca | jam: --durations gives you something like | 10:13 |
Chipaca | 2.24s call test/test_main.py::TestMainWithNoDispatchButJujuIsDispatchAware::test_multiple_events_handled | 10:13 |
Chipaca | 2.21s call test/test_main.py::TestMainWithNoDispatch::test_setup_event_links | 10:13 |
Chipaca | etc | 10:13 |
Chipaca | jam: also, pip install pytest-xdist and then pytest -n $(nproc) | 10:13 |
jam | Chipaca, sure. The issue is that I know one test is running slow, and I'm trying to figure out why | 10:14 |
Chipaca | ahh | 10:15 |
Chipaca | you want timings _inside_ the test? | 10:15 |
Chipaca | that i don't have | 10:15 |
jam | Chipaca, yeah. Which --profiling gives me. the issue is that one of my tests with SQLite backend is about 3ms, and with Juju backend it is about 600ms | 10:15 |
jam | I'd like to understand why :) | 10:15 |
Chipaca | jam: did you set SSH_TO_THUMPERS_BOX=0 ? | 10:16 |
Chipaca | 600ms is enough time to do that and more … :) | 10:17 |
Chipaca | jam: what happens if you set PYTHONVERBOSE in the environ? | 10:17 |
jam | 376ms spent in subprocess.communicate, 376ms spent in wait() | 10:19 |
jam | stupid process barriers | 10:19 |
jam | Chipaca, PYTHONVERBOSE is a bit... verbose? :) | 10:20 |
jam | Chipaca, stripping it down to a single emit() shows it as 200ms, https://paste.ubuntu.com/p/XY7dmtXNxv/ | 10:25 |
Chipaca | jam: is something reading stdin/stdout with it being buffered on a timeout? | 10:26 |
jam | Chipaca, it is about 20m-30ms per subprocess invocation, we just do a lot of them for each emit() | 10:26 |
jam | Chipaca, $ time python3 -c '' | 10:27 |
jam | real 0m0.015s | 10:27 |
Chipaca | :-( | 10:27 |
Chipaca | jam: try -Sc | 10:27 |
jam | goes down to 7ms | 10:28 |
Chipaca | (not practical irl but good to know how low we could take it with some environ handholding) | 10:28 |
jam | Chipaca, gives "No module named 'yaml'" if I try it in the script | 10:29 |
Chipaca | yeah, as i say, not practical :) | 10:29 |
Chipaca | it comes up with a very empty pythonpath, for one | 10:29 |
jam | Chipaca, it works for the ones that don't need yaml, though | 10:30 |
jam | but that only shaves ~10ms off the test time | 10:31 |
Chipaca | jam: what are all the calls the reemit does? | 10:32 |
jam | so emit() calls _emit(), which does a 'save_snapshot()' for the event, then 'save_notice()' for each observer. | 10:33 |
Chipaca | and each of those is a save-set | 10:33 |
jam | then _reemit calls 'read all notices', and for each one it will then call load_snapshot and possibly drop_snapshot/drop_notice | 10:34 |
Chipaca | jam: right | 10:34 |
Chipaca | jam: sounds to me like we want to consider refactoring things to at least save in a batch | 10:35 |
Chipaca | but later | 10:35 |
Chipaca | i mean, let's not block save-set on this :) | 10:35 |
jam | so I think in practice it won't be as bad, 'juju run 'time state-get' is 5ms vs 20ms | 10:35 |
Chipaca | but let's not call save-set *done* without looking at it | 10:35 |
jam | but it is still a lot slower than interacting with an in-process SQLite file | 10:36 |
* Chipaca reaches for his 'shocked pikachu' face again | 10:36 | |
jam | Chipaca, 😲 | 10:37 |
Chipaca | :-) that's the on | 10:37 |
jam | 😱 | 10:37 |
Chipaca | nah that's the dog :-p | 10:37 |
jam | Chipaca, I do think given how much the Operator framework is based around events, we'll want to be cautious about its performance | 10:38 |
Chipaca | (somebody pointed it 'the scream' kinda looks like the guy tried to draw a dog and then somebody said 'oh wow a person screaming' and they went with it) | 10:38 |
jam | https://emojipedia.org/face-screaming-in-fear/ I'm not seeing the dog | 10:38 |
jam | nor here: https://www.bookdepository.com/Edvard-Munch-Masterpieces-of-Art-Candice-Russell/9781783613564?redirected=true&utm_medium=Google&utm_campaign=Base3&utm_source=AE&utm_content=Edvard-Munch-Masterpieces-of-Art&selectCurrency=AED&w=AFCFAU968L892MA8VCPZ&gclid=Cj0KCQjwoaz3BRDnARIsAF1RfLeKR4-ljbzhpZgajGSi2f4h-DLQp_FaX-dDHKqVeiWmSi23iue6HVAaAlf2EALw_wcB | 10:38 |
jam | Chipaca, ah, the hands are the ears | 10:39 |
Chipaca | there you go :) | 10:39 |
Chipaca | once you see it it's easy to flip-flop | 10:39 |
jam | if you get rid of the nose and mouth, then I can see the dog nose, but I don't see how they fit on the dog face | 10:40 |
jam | not as good a flip/flop as https://www.independent.co.uk/news/science/duck-or-rabbit-the-100-year-old-optical-illusion-that-tells-you-how-creative-you-are-a6873106.html | 10:40 |
jam | duck bunny | 10:40 |
Chipaca | the duck/bunny one is very good (i first saw it on a joke) | 10:41 |
Chipaca | anyway. pop. | 10:42 |
Chipaca | facundo__: so, wrt charmcraft and secrets, i think a reasonable approach is to use keyring and have the snap ask for the secrets plug; it won't auto-connect, but we can detect the failure in code and fall back to plaintext (or notify the user to manually connect or pass --plaintext?) | 10:44 |
Chipaca | the token isn't a general-purpose do-anything token fwiw, which is why storing it plaintext isn't _that_ bad an idea | 10:45 |
jam | 'time python3 -c import yaml' is 30ms here.. :( | 10:49 |
Chipaca | 0.05user 0.00system 0:00.06elapsed 98%CPU (0avgtext+0avgdata 11216maxresident)k | 10:50 |
jam | 50-60ms ? | 10:51 |
Chipaca | yep | 10:52 |
Chipaca | and 11MB | 10:52 |
facundo__ | Muy buenos días a todos! | 10:54 |
=== facundo__ is now known as facubatista | ||
Chipaca | jam: of course that's slower in great part because my cpu aggressively freqs down; if i pin it hot it drops to 30ms | 10:55 |
Chipaca | and if i were to enable 'turbo' mode it'd probably drop another ~5ms | 10:55 |
Chipaca | but i don't like my fans being on loud all the time =) | 10:55 |
Chipaca | facubatista: muyy buen día su señoría mantantirulirulá | 10:56 |
facubatista | Chipaca, supercalifragilísticoespialidosas mañanas! | 10:57 |
jam | Chipaca, thinking about Ajduk... It came to light that Operators can trigger an action to be run, which means the Operator charm *should* be able to cause a script to run in the application container | 11:26 |
jam | Chipaca, https://discourse.juju.is/t/coordinating-actions-for-a-k8s-operators/3161/2 | 11:27 |
jam | Chipaca, I added a comment to the doc | 11:30 |
facubatista | Chipaca, https://github.com/canonical/charmcraft/issues/42 | 11:31 |
jam | Chipaca, facubatista : all wired up! turns out we aren't very good at cleaning up after ourselves: | 11:41 |
jam | $ juju run --unit uo/2 'state-get' | 11:41 |
jam | '#notices#': | | 11:41 |
jam | [] | 11:41 |
jam | StoredStateData[_stored]: | | 11:41 |
jam | {event_count: 12} | 11:41 |
jam | Ubuntu/on/config_changed[7]: | | 11:41 |
jam | null | 11:41 |
jam | ... | 11:41 |
jam | Ubuntu/on/install[1]: | | 11:41 |
jam | null | 11:41 |
jam | ... | 11:41 |
jam | Ubuntu/on/leader_elected[4]: | | 11:41 |
jam | null | 11:41 |
jam | ... | 11:41 |
jam | on/commit[3]: | | 11:41 |
jam | null | 11:41 |
jam | ... | 11:41 |
jam | on/commit[6]: | | 11:41 |
jam | null | 11:41 |
jam | ... | 11:41 |
jam | on/commit[9]: | | 11:41 |
jam | null | 11:41 |
jam | ... | 11:41 |
jam | on/commit[12]: | | 11:41 |
jam | null | 11:41 |
jam | ... | 11:42 |
jam | on/pre_commit[2]: | | 11:42 |
jam | null | 11:42 |
jam | ... | 11:42 |
jam | on/pre_commit[5]: | | 11:42 |
jam | null | 11:42 |
jam | ... | 11:42 |
jam | on/pre_commit[8]: | | 11:42 |
jam | null | 11:42 |
jam | ... | 11:42 |
jam | on/pre_commit[11]: | | 11:42 |
jam | null | 11:42 |
jam | https://paste.ubuntu.com/p/NWZy54VMk4/ | 11:42 |
jam | (didn't mean to paste the content here) | 11:42 |
facubatista | :) | 11:42 |
Chipaca | i'll be 10 minutes late for the revue | 11:51 |
Chipaca | (small lunch chaos here) | 11:51 |
jam | Chipaca, facubatista so the bug is that events that have no handlers are always treated as deferred | 11:51 |
facubatista | oh | 11:52 |
* facubatista plans the meeting for 10' past normal time | 11:55 | |
Chipaca | jam: that's a nasty bug :-| | 12:12 |
facubatista | Chipaca, https://discourse.juju.is/t/how-to-build-a-charm-using-modern-tools/3246 | 12:13 |
mup | Issue operator#333 opened: Events with no observers never get deleted <Created by jameinel> <https://github.com/canonical/operator/issues/333> | 12:23 |
Chipaca | what's the generic name for what 'juju deploy' deploys to? | 13:07 |
facubatista | Chipaca, the "backing cloud"? | 13:13 |
Chipaca | facubatista: hmm | 13:14 |
Chipaca | facubatista: in the end i rewrote the sentence to not need it, fwiw | 13:14 |
Chipaca | facubatista: https://discourse.juju.is/t/what-files-would-you-expect-charmcraft-build-copies-into-the-charm/3247 | 13:14 |
Chipaca | stub: mthaddon: ^ if you have opinions on it please speak up :) | 13:14 |
mthaddon | ack, will try and take a look soon | 13:38 |
mup | PR operator#334 opened: Unobserved events <Created by jameinel> <https://github.com/canonical/operator/pull/334> | 14:14 |
jam | Chipaca, PRs should be up for review | 14:15 |
jam | facubatista, review of charmcraft is up | 14:15 |
facubatista | jam, thanks | 14:15 |
Chipaca | jam: which are the python implementations of state-set that are slow? where can i play with that? | 14:15 |
Chipaca | s/set/[sg]et/ fwiw | 14:17 |
facubatista | jam, what do you mean with "make it contextual"? | 14:18 |
Chipaca | facubatista: pass it around either explicitly or implicity, i think | 14:19 |
facubatista | it could be done | 14:23 |
facubatista | I like it how it's now, though, as it should be an omnipresent object, the only interaction with the module, etc | 14:24 |
jam | Chipaca, https://github.com/canonical/operator/pull/323/files#diff-54d96dff01765335f1f08f6254aa0de6R197-R250 is the python backend | 15:08 |
mup | PR #323: 317 state get <Created by jameinel> <https://github.com/canonical/operator/pull/323> | 15:08 |
jam | facubatista, yeah, the idea was 'is it worth passing one around' rather than being used as a process global | 15:08 |
Chipaca | heh, silly me was looking for #! :) | 15:14 |
jam | Chipaca, yeah, the #! is fixed to be bash by fake_script | 15:40 |
mup | PR operator#335 opened: make tests 15% faster <Created by chipaca> <https://github.com/canonical/operator/pull/335> | 16:03 |
Chipaca | :-D | 16:04 |
Chipaca | tomorrow is RTD DAY \o/ | 16:33 |
Chipaca | ok, i think i'm at eod, mostly | 16:57 |
Chipaca | facubatista: i'm still hoping to finish the review on charmcraft#33 tonight though | 16:58 |
mup | PR charmcraft#33: Refactored all the message sending to the user, including storing everything in a file <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/33> | 16:58 |
Chipaca | but that'll depend on external factors :) | 16:58 |
facubatista | Chipaca, tomorrow is fine, too | 17:00 |
mup | PR operator#335 closed: make tests 15% faster <Created by chipaca> <Merged by chipaca> <https://github.com/canonical/operator/pull/335> | 20:25 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!