/srv/irclogs.ubuntu.com/2021/06/23/#snappy.txt

mborzeckimorning05:06
mardymborzecki: hi!05:55
mborzeckimardy: heya05:55
pstolowskimorning07:07
mardyI'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:14
mardy        imports github.com/snapcore/snapd/overlord/snapstate07:15
mardy        imports github.com/snapcore/snapd/overlord/snapstate/policy07:15
mardy        imports github.com/snapcore/snapd/overlord/snapstate07:15
mardyin 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:17
zyga-mbpgood morning07:20
zyga-mbpnew sprint starts for me today, I will have time to finish spread shellcheck converter (and the lava converter)07:20
zyga-mbphey pedronis :)07:20
zyga-mbphow are you doing?07:20
mborzeckimardy: hey, can you take a look at https://github.com/snapcore/snapd/pull/10444 ?07:27
mardymborzecki: +1!07:44
mborzeckithx07:44
zyga-mbpwhat's up with mup?08:11
zyga-mbpno more notices on irc/08:11
zyga-mbpdid mup survive freenode apocalypse?08:11
zyga-mbpoh, mup is gone08:11
zyga-mbpI guess mup is the last user on freenode now :)08:11
pedronispstolowski: I reviewed https://github.com/snapcore/snapd/pull/10408 , thanks for the changes08:23
pstolowskipedronis: thanks!08:25
mardypedronis: 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 package08:33
pedronis:/08:34
mardypedronis: I've yet to investigate this deeply, but my first impression is that the policy package should not depend on the snapstate package08:34
pedronisit's messy, I forgot08:34
mardyI'm now going to check what are the things that policy requires from snapstate, and then we can decide how to proceed08:35
pedronismardy: sorry, let me think a second, there might something easier that can we do08:39
pedronishttps://github.com/snapcore/snapd/pull/10442 needs reviews, it's a bit convoluted but it's a refactoring essentially, the real change will come later08:40
mardypedronis: so, the policy package uses SnapState, Policy, PolicyFor, NumSnaps, All, Flags, Set from the snapstate package. That's quite a lot of stuff.08:45
pedronismardy: just make a snap/system_usernames.go instead08:52
=== pedronis_ is now known as pedronis
mardypedronis: thanks :-)09:07
mborzeckianyone remembers where was the spread binary that supported `-workers <n>` ?10:01
mborzeckistore seems to be a bit unhappy today10:14
mborzeckiseeing `error: cannot refresh "lxd": unexpectedly empty response from the server (try again later)`10:15
mborzeckiehh a bit of a bummer with current tags10:38
zyga-mbphaha10:39
zyga-mbpit was the one from cachio10:39
zyga-mbpcustom build with more patches for usability10:40
pstolowskimborzecki: can you take a look at this PR from cachio? needs 2nd review: https://github.com/snapcore/snapd/pull/1042410:40
pedronispstolowski: I put now https://github.com/snapcore/snapd/pull/7700 into my review queue10:44
pstolowskity10:45
mborzeckimeh, 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 all10:48
mborzeckithere'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 xenial10:50
mardyam 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?10:50
zyga-mbpmardy looking11:04
zyga-mbplooks like store one one failure11:06
zyga-mbpand I think the apt hooks are problematic, I recall reading about that recently11:06
zyga-mbpI did not read the diff but I think it is unlikely master passes11:06
mardyzyga-mbp: thanks, I'll retry it later then11:18
zyga-mbpmardy sadly spread rarely passes in snapd11:19
zyga-mbpas tests are a bit too integration testing sometimes11:19
zyga-mbpand picking up all kinds of motion in the ecosystem11:19
zyga-mbpdistros and the store alike11:19
mardypedronis: 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-L1311:37
pedronismardy: you need to involve the fake store I fear to test this, otherwise you'll have no snap-ids11:38
mardypedronis: ah, I didn't know we had a fake store for the tests! Sounds cool :-)11:39
pedronismardy: I think we have a similar case already, let me check11:39
pedronismardy: devicestate.CanManageRefreshes is similar in the sense it has a check that depends on the snap having a snap-id11:41
pedronismardy: so I think the spread test for it should have at least some inspiration:  ./tests/main/interfaces-snapd-control-with-manage/task.yaml11:42
pedronismborzecki: I made some high-level comments in https://github.com/snapcore/snapd/pull/1043611:43
mborzeckithanks11:43
pedronismborzecki: added another one11:49
pedronispstolowski: I think we forgot to consider something related to error handling: https://github.com/snapcore/snapd/pull/10408#discussion_r65703650012:17
pedronisunless I'm confused12:17
pstolowskipedronis: dang, you are right, runHook will call our handler but then error the task12:21
pstolowskipedronis: ok as a followup?12:22
pedronispstolowski: yes, but we need to decide a bit what to do there12:23
pedronispstolowski: I think we have very few hooks (handlers) fwiw that have non-trivial Error behavior12:26
pstolowskipedronis: maybe even none?12:30
pedronispstolowski: healthstate I think does12:30
pstolowskiindeed12:31
pedronispstolowski: should we chat quickly about this now or after the standup?12:40
pstolowskipedronis: sorry, was a bit distracted, maybe after standup then?12:58
pedronispstolowski: yes13:01
pstolowskimardy: i added a general comment https://github.com/snapcore/snapd/pull/10438/files#r657180723, hope it makes sense14:52
mardypstolowski: ah! Didn't think of that! Thanks, will find another way.15:00

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!