[06:13] <mup> PR snapd#9586 opened: update-pot: include file locations in translation template, and extract strings from desktop files <Created by jhenstridge> <https://github.com/snapcore/snapd/pull/9586>
[07:09] <mup> PR snapd#9587 opened: snap: fix failing unittest for quantity.FormatDuration() <⚠ Critical> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9587>
[07:14] <mborzecki> morning
[07:14] <jamesh> Do we have a unit test that assumes all months have 30 days?
[07:15] <zyga> good morning
[07:16] <zyga> mvo, with samuele away for the part of the week, 9546 won't make official progress but perhaps it could be reviewed by others to spot any mistakes or weirdness
[07:16] <jamesh> It looks like the TestSnapshotImportHappy test was written to start failing on November 1st
[07:16] <zyga> I suspect the changes are acceptable by him now, he just ran out of review time on Friday
[07:17] <zyga> hey jamesh :)
[07:17] <jamesh> hi zyga
[07:19] <mborzecki> zyga: jamesh: hey
[07:22] <mvo> good morning mborzecki and zyga
[07:23] <mborzecki> mvo: hey
[07:23] <mvo> and good afternoon jamesh :)
[07:23] <mvo> zyga: I will try to have a look
[07:23] <mvo> jamesh: I pushed a PR to fix the failing unittest
[07:23] <mvo> jamesh, mborzecki PR#9587 fwiw
[07:24] <zyga> mvo, thanks, no rush but I think making that landable will allow the rest to be reviewed in parallel
[07:26] <zyga> mvo, https://github.com/snapcore/snapd/pull/9204 is green and has +2 but should be acked by someone the last time
[07:26] <mup> PR #9204: sandbox: track applications unconditionally <Created by zyga> <https://github.com/snapcore/snapd/pull/9204>
[07:26] <zyga> jamesh, I think you will like that ^
[07:26] <zyga> it makes snap identification work on Fedora and elsewhere
[07:26] <zyga> where we have cgroupv2
[07:28] <jamesh> mvo: might it be better to remove the check entirely?  Your fix looks like it will only help up until March
[07:29] <mvo> jamesh: not opposed to remove it, it's the wrong layer for this test anyway
[07:34] <mvo> mborzecki: how do you feel about merging 9582?
[07:51] <mborzecki> mvo: sounds good to me, i can open a followup with test name tweak
[08:02] <pstolowski> morning
[08:02] <mvo> good morning pstolowski
[08:03] <zyga> hey pstolowski :)
[08:05] <pstolowski> o/
[08:05] <mborzecki> mvo: if you don't mind i'll push a patch dropping the check competely to #9587
[08:05] <mup> PR #9587: snap: fix failing unittest for quantity.FormatDuration() <Skip spread> <⚠ Critical> <Created by mvo5> <https://github.com/snapcore/snapd/pull/9587>
[08:05] <mvo> mborzecki: please do
[08:14] <mup> PR snapd#9588 opened: dirs: add "gentoo" to altDirDistros <Created by zmedico> <https://github.com/snapcore/snapd/pull/9588>
[08:19] <zyga> mborzecki, https://github.com/snapcore/snapd/pull/9588#pullrequestreview-521409043
[08:19] <mup> PR #9588: dirs: add "gentoo" to altDirDistros <Created by zmedico> <https://github.com/snapcore/snapd/pull/9588>
[08:19] <zyga> FYI
[08:24] <mup> PR snapd#9582 closed: gadget/quantity: introduce a new package that captures quantities  <Simple 😃> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9582>
[08:34] <mup> PR snapd#9589 opened: gadget/quantity: tweak test name <Simple 😃> <Skip spread> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9589>
[08:54] <mup> PR snapd#9587 closed: snap: fix failing unittest for quantity.FormatDuration() <Skip spread> <⚠ Critical> <Created by mvo5> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9587>
[09:24]  * dot-tobias says hi
[09:41] <mvo> can someone check 9560 please? needs a second review, maybe mborzecki  ?
[09:42] <mborzecki> mvo: i have it open, just checkign some things that ondra raised
[09:43] <mvo> mborzecki: sure, thank you! no rush
[09:45] <mvo> zyga: nice, 9204 is green now \o/
[09:54] <mup> PR snapd#9589 closed: gadget/quantity: tweak test name <Simple 😃> <Skip spread> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9589>
[10:08] <niemeyer> Heya
[10:08] <niemeyer> Good morning all
[10:08] <niemeyer> pstolowski: Around?
[10:09] <pstolowski> niemeyer: hey, yes
[10:09] <niemeyer> pstolowski: Do you have a moment to explain the task-id/change-id presence in the timing module?
[10:11] <niemeyer> pstolowski: Even ignoring the encapsulation concern (timing knows about details of tasks/changes usage of itself), it seems curious that this logic takes effect precisely when timings are empty
[10:18] <pstolowski> niemeyer: yes. 1 - we enrich the data returned to snap debug timings with doing/undoing times obtained from tasks, and we combine this data when queried; 2 - for change-id, we want to correlate a change with ensure loop that triggered it
[10:19] <niemeyer> pstolowski: Yeah, I understand the idea of tagging. Seems sound. I don't understand the idea of preserving a timing that has no timings.
[10:22] <pstolowski> niemeyer: i don't think this is the case -  see func (t *Timings) Save(s GetSaver) line 140 & 141, and flatten() line 85
[10:23] <niemeyer> 	if len(t.timings) == 0 && !isEnsureWithChange {
[10:23] <niemeyer> 		return nil
[10:23] <niemeyer> 	}
[10:25] <niemeyer> pstolowski: The only use cases I can identify are clearly being used only to prevent wiping out empty timings
[10:29] <pstolowski> niemeyer: right,  i see what you mean, i need to think what was the driver for this change
[10:29] <niemeyer> pstolowski: If it's irrelevant, that's a nice cleanup. Besides simplifying the logic and making it less obscure, it also takes away the only reason why there's breakage of encapsulation there.
[10:31] <niemeyer> pstolowski: There's also a potential change in API that would reduce the API surface of the module. But that's a more involved improvement and that isn't really fixing any clear structural issues.
[10:35] <pstolowski> niemeyer: so related PR is https://github.com/snapcore/snapd/pull/6904 and second paragraph of the description summarizes the reason for keeping empty timing in this case; there was a small followup https://github.com/snapcore/snapd/pull/6921 where I unfortunately dropped the comment that was useful
[10:35] <mup> PR #6904: timings: always store ensure timings as long as they have an associated change <Created by stolowski> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6904>
[10:35] <mup> PR #6921: timings: tweak the conditional for ensure timings <Simple 😃> <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/6921>
[10:39] <niemeyer> pstolowski: Ah, it's nice to have the background, thanks. It still seems suboptimal, though.. you have two pieces of logic inside the module fighting with each other, and in a pretty convoluted manner, requiring knowledge of completely unrelated things.
[10:39] <niemeyer> pstolowski: That is,
[10:40] <niemeyer> pstolowski: There's one thing that removes fast timings, one thing that removes top-level empty timings, and one thing that prevents both of these from working.
[10:40] <niemeyer> pstolowski: If top-level timings are relevant even if they are empty, then we might just keep them around.
[10:41] <niemeyer> pstolowski: If fast timings are still relevant because people might search for them, we might just keep them around as well.
[10:41] <niemeyer> pstolowski: Or reduce the threshold until it's more honest
[10:42] <niemeyer> Either way, thanks for the background. That's indeed what I was looking for and now I know how to proceed with the skeleton here. This is not super important for snapd, so I'll let you decide whether/when to look into it.
[10:43] <pstolowski> niemeyer: yes, i see how it is convoluted.  the only concern is not to  exhaust our timings "buffer" in the state, we want to keep a limited number of them, and we want only those that are time-consuming, so we don't want empty timings there in most cases
[10:45] <niemeyer> pstolowski: Sounds like the code disagrees. You've made a decision based on UX to actually have non-time consuming things around to satisfy user searches.
[10:47] <niemeyer> pstolowski: Proper encapsulation would try to understand that requirement and design for it, instead of teaching the module to do guess work based on deep knowledge of things far away.
[10:52] <pstolowski> niemeyer: no disagreement. in the defence of that, this was initially all about task/changes (and imported state). a year later it was separated with https://github.com/snapcore/snapd/pull/8283
[10:52] <mup> PR #8283: overlord,timings,daemon: separate timings from overlord/state <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/8283>
[10:56] <niemeyer> pstolowski: What I see there is Samuele working exactly on the direction being suggested here, which is clean up the encapsulation
[10:56] <pstolowski> unbelievable, it's 2 years since timings were introduced, time flies...
[10:56] <niemeyer> pstolowski: This is a top-level package..
[10:56] <niemeyer> Yeah, timing flies
[10:57] <pstolowski> niemeyer: yes.. although the driver had a different angle as mentioned on mattermost... we wanted to not import state package elsewhere
[10:57] <niemeyer> pstolowski: Not importing the state package only has a meaning if you're not marrying with its internals :)
[10:58] <pstolowski> yes sure
[11:31] <mvo> mborzecki: 9577 looks good to me now, wdyt?
[11:37] <mvo> mborzecki: I fixed one missing test update in 9560 and then I think this can land :) thanks for the review!
[11:38] <mborzecki> mvo: i've done the followup changes, but having some trouble with mocking sfdisk properly
[12:01] <pstolowski> mvo: i've a spread test for download timeout, slightly tricky stuff
[12:01] <pstolowski> see #9590
[12:01] <mup> PR #9590: tests: download timeout spread test <Created by stolowski> <https://github.com/snapcore/snapd/pull/9590>
[12:02] <mvo> pstolowski: nice
[12:04] <mvo> pstolowski: just looked, really cool
[12:05] <pstolowski> mvo: let's see if it's not flaky or something.. i only run it on 20.04
[12:05] <mup> PR snapd#9590 opened: tests: download timeout spread test <Created by stolowski> <https://github.com/snapcore/snapd/pull/9590>
[12:45] <mup> PR snapd#9560 closed: gadget/many: drop usage of gpt attr 59 for indicating creation of partitions <Run nested> <UC20> <Created by anonymouse64> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/9560>
[12:45] <mup> PR snapd#9591 opened: gadget, gadget/install: move helpers to install package, refactor unit tests <Run nested> <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9591>
[12:45] <mborzecki> mvo: ^^
[13:34] <mborzecki> mvo: pushed some changes to #9577
[13:34] <mup> PR #9577: many: seal a fallback object to the recovery boot chain <Run nested> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9577>
[13:57] <mvo> mborzecki: \o/ thank you
[14:38] <ijohnson> mvo: I also think we need https://github.com/snapcore/snapd/pull/9541 for 2.48, but we are still blocked on moving forward with that :-/
[14:38] <mup> PR #9541: osutil/disks: re-implement partition searching for disk w/ non-adjacent parts <Bug> <Run nested> <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/9541>
[14:39] <mvo> ijohnson: let's raise it again today
[14:39] <ijohnson> sounds good
[14:39]  * mvo puts it on the meeting agenda
[15:09]  * cachio lunch
[15:14] <mup> PR snapcraft#3340 closed: include SNAPCRAFT_STAGE in XDG_DATA_DIRS, needed to find Gir files built by other parts <Created by kenvandine> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3340>
[15:15] <mup> Issue core20#94 opened: fix pi4 names for wlan0 and eth0 <Created by xnox> <https://github.com/snapcore/core20/issues/94>
[15:47] <mborzecki> need to drive the kids a bit
[15:58] <ijohnson> mvo: submitted review for 9577
[15:59] <mvo> ijohnson: thanks!
[15:59] <ijohnson> np
[15:59] <ijohnson> I will work on my followup to use that logic in snap-bootstrap today as we discussed
[16:02] <mvo> ta
[16:15] <pstolowski> quick errand, bbiab
[16:55] <mvo> cachio: I see failures like: "2020-11-02 16:00:35 Cannot allocate google-nested:ubuntu-18.04-64: cannot find any Google image matching "ubuntu-1804-64-virt-enabled" on project "computeengine" or "ubuntu-os-cloud"" in pr 9591, is there some gce issue?
[16:55] <mup> PR #9591: gadget, gadget/install: move helpers to install package, refactor unit tests <Run nested> <Simple 😃> <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/9591>
[16:55] <ijohnson> mvo: cachio I think the issue is that there are too many images in our gce project, usually cachio just goes in and clears out some old images and it works again
[16:55] <cachio> mvo, let me cehck, I think I need to clean up
[16:56] <mvo> ijohnson: oh, right :(
[16:56] <ijohnson> because gce api is silly and still doesn't know how to return more than X images if there are more than X images in the project
[16:57] <cachio> ijohnson, well, they have pagination to support taht
[16:57] <cachio> but spread does not support pagination
[16:58] <mvo> ijohnson: I noticed two small issues around 9577 https://github.com/snapcore/snapd/pull/9577#discussion_r516113171 and https://github.com/snapcore/snapd/pull/9577#discussion_r516062913 otherwise this is good to go
[16:58] <mup> PR #9577: many: seal a fallback object to the recovery boot chain <Run nested> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9577>
[16:58] <ijohnson> ah I see
[16:58] <ijohnson> mvo: looking now
[16:58] <mvo> ijohnson: \o/
[16:58] <mvo> ijohnson: spread> there are two draft PRs for spread to support pagination but both not quite get it right :/
[16:59] <ijohnson> ah I see
[16:59] <ijohnson> I thought this was gce's fault
[17:00] <cachio> mvo, do you want to re-trigger
[17:00] <cachio> mvo, already cleaned up
[17:01] <ijohnson> mvo: re 9577, do you mean in `TestMakeBootable20RunModeSealKeyErr` that ResealKeys (or SealKeys? I forget which one) is never actually called?
[17:01] <pstolowski> re
[17:06] <mvo> ijohnson: sorry, I mean in https://github.com/snapcore/snapd/pull/9577#discussion_r516113171 that there is no check like "c.Assert(sealKeysCalls, Equals, 2)" (or 1)
[17:06] <mup> PR #9577: many: seal a fallback object to the recovery boot chain <Run nested> <UC20> <Created by cmatsuoka> <https://github.com/snapcore/snapd/pull/9577>
[17:06] <ijohnson> mvo: ah yes you are right that is true
[17:06] <ijohnson> that is less of an issue than I thought you were trying to say :-)
[17:06] <mvo> ijohnson: it also already fails after the first call with an error so it will never be true in the switch
[17:07] <mvo> ijohnson: it also means we don't actually test the error transmission from the second call :)
[17:07] <mvo> ijohnson: it's probably not much of an issue, I agreee
[17:12]  * mvo gets dinner
[17:39] <timothy> hi zyga, is confinement broken on Fedora 33?
[17:40] <timothy> https://paste.centos.org/view/e5b46b5b