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