[05:06] <mborzecki> morning
[05:55] <mardy> mborzecki: hi!
[05:55] <mborzecki> mardy: heya
[07:07] <pstolowski> morning
[07:14] <mardy> I'm trying to implement Samuele's suggestion from https://github.com/snapcore/snapd/pull/10375#discussion_r656277940, but I'm now getting an error about a cyclic dependency:
[07:15] <mardy>         imports github.com/snapcore/snapd/overlord/snapstate
[07:15] <mardy>         imports github.com/snapcore/snapd/overlord/snapstate/policy
[07:15] <mardy>         imports github.com/snapcore/snapd/overlord/snapstate
[07:17] <mardy> in general, in golang, in which directions should dependency go? from the packages residing in the deeper directory to the more top-level packages? Or is there another logic altogether?
[07:20] <zyga-mbp> good morning
[07:20] <zyga-mbp> new sprint starts for me today, I will have time to finish spread shellcheck converter (and the lava converter)
[07:20] <zyga-mbp> hey pedronis :)
[07:20] <zyga-mbp> how are you doing?
[07:27] <mborzecki> mardy: hey, can you take a look at https://github.com/snapcore/snapd/pull/10444 ?
[07:44] <mardy> mborzecki: +1!
[07:44] <mborzecki> thx
[08:11] <zyga-mbp> what's up with mup?
[08:11] <zyga-mbp> no more notices on irc/
[08:11] <zyga-mbp> did mup survive freenode apocalypse?
[08:11] <zyga-mbp> oh, mup is gone
[08:11] <zyga-mbp> I guess mup is the last user on freenode now :)
[08:23] <pedronis> pstolowski: I reviewed https://github.com/snapcore/snapd/pull/10408 , thanks for the changes
[08:25] <pstolowski> pedronis: thanks!
[08:33] <mardy> pedronis: hi! I don't know if you can see my question above, but when moving the system users to the policy package, I'm getting into a cyclic dependency because the policy package also depends on the snapstate package
[08:34] <pedronis> :/
[08:34] <mardy> pedronis: I've yet to investigate this deeply, but my first impression is that the policy package should not depend on the snapstate package
[08:34] <pedronis> it's messy, I forgot
[08:35] <mardy> I'm now going to check what are the things that policy requires from snapstate, and then we can decide how to proceed
[08:39] <pedronis> mardy: sorry, let me think a second, there might something easier that can we do
[08:40] <pedronis> https://github.com/snapcore/snapd/pull/10442 needs reviews, it's a bit convoluted but it's a refactoring essentially, the real change will come later
[08:45] <mardy> pedronis: so, the policy package uses SnapState, Policy, PolicyFor, NumSnaps, All, Flags, Set from the snapstate package. That's quite a lot of stuff.
[08:52] <pedronis> mardy: just make a snap/system_usernames.go instead
[09:07] <mardy> pedronis: thanks :-)
[10:01] <mborzecki> anyone remembers where was the spread binary that supported `-workers <n>` ?
[10:14] <mborzecki> store seems to be a bit unhappy today
[10:15] <mborzecki> seeing `error: cannot refresh "lxd": unexpectedly empty response from the server (try again later)`
[10:38] <mborzecki> ehh a bit of a bummer with current tags
[10:39] <zyga-mbp> haha
[10:39] <zyga-mbp> it was the one from cachio
[10:40] <zyga-mbp> custom build with more patches for usability
[10:40] <pstolowski> mborzecki: can you take a look at this PR from cachio? needs 2nd review: https://github.com/snapcore/snapd/pull/10424
[10:44] <pedronis> pstolowski: I put now https://github.com/snapcore/snapd/pull/7700 into my review queue
[10:45] <pstolowski> ty
[10:48] <mborzecki> meh, so it's possible that TAGS is non null, while CURRENT_TAGS is null, eg. when the current set of tags does not match the device anymore, in which case it's impossible to distinguish a case where we're dealing with newer systemd/udev and there are no tags, or the old one where the CURRENT_TAGS did not exist at all
[10:50] <mborzecki> there's a call in libsystemd device_database_supports_current_tags but it isn't exported, and it's only part of the newer libsystemd, while our builds of snapd snap are done on xenial
[10:50] <mardy> am I right in assuming that the spread failures in https://github.com/snapcore/snapd/pull/10438/checks?check_run_id=2891955009 are not due to my changes?
[11:04] <zyga-mbp> mardy looking
[11:06] <zyga-mbp> looks like store one one failure
[11:06] <zyga-mbp> and I think the apt hooks are problematic, I recall reading about that recently
[11:06] <zyga-mbp> I did not read the diff but I think it is unlikely master passes
[11:18] <mardy> zyga-mbp: thanks, I'll retry it later then
[11:19] <zyga-mbp> mardy sadly spread rarely passes in snapd
[11:19] <zyga-mbp> as tests are a bit too integration testing sometimes
[11:19] <zyga-mbp> and picking up all kinds of motion in the ecosystem
[11:19] <zyga-mbp> distros and the store alike
[11:37] <mardy> pedronis: any idea how I could test the snapId check? With the latest changes, all local snaps can be installed (since we use --dangerous), so I'm struggling to find a way to test this: https://github.com/anonymouse64/snapd/blob/feature/snap-microk8s-shared-system-usernames/tests/main/system-usernames-microk8s/task.yaml#L7-L13
[11:38] <pedronis> mardy: you need to involve the fake store I fear to test this, otherwise you'll have no snap-ids
[11:39] <mardy> pedronis: ah, I didn't know we had a fake store for the tests! Sounds cool :-)
[11:39] <pedronis> mardy: I think we have a similar case already, let me check
[11:41] <pedronis> mardy: devicestate.CanManageRefreshes is similar in the sense it has a check that depends on the snap having a snap-id
[11:42] <pedronis> mardy: so I think the spread test for it should have at least some inspiration:  ./tests/main/interfaces-snapd-control-with-manage/task.yaml
[11:43] <pedronis> mborzecki: I made some high-level comments in https://github.com/snapcore/snapd/pull/10436
[11:43] <mborzecki> thanks
[11:49] <pedronis> mborzecki: added another one
[12:17] <pedronis> pstolowski: I think we forgot to consider something related to error handling: https://github.com/snapcore/snapd/pull/10408#discussion_r657036500
[12:17] <pedronis> unless I'm confused
[12:21] <pstolowski> pedronis: dang, you are right, runHook will call our handler but then error the task
[12:22] <pstolowski> pedronis: ok as a followup?
[12:23] <pedronis> pstolowski: yes, but we need to decide a bit what to do there
[12:26] <pedronis> pstolowski: I think we have very few hooks (handlers) fwiw that have non-trivial Error behavior
[12:30] <pstolowski> pedronis: maybe even none?
[12:30] <pedronis> pstolowski: healthstate I think does
[12:31] <pstolowski> indeed
[12:40] <pedronis> pstolowski: should we chat quickly about this now or after the standup?
[12:58] <pstolowski> pedronis: sorry, was a bit distracted, maybe after standup then?
[13:01] <pedronis> pstolowski: yes
[14:52] <pstolowski> mardy: i added a general comment https://github.com/snapcore/snapd/pull/10438/files#r657180723, hope it makes sense
[15:00] <mardy> pstolowski: ah! Didn't think of that! Thanks, will find another way.