mborzecki | morning | 05:06 |
---|---|---|
mardy | mborzecki: hi! | 05:55 |
mborzecki | mardy: heya | 05:55 |
pstolowski | morning | 07:07 |
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:14 |
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:15 |
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:17 |
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:20 |
mborzecki | mardy: hey, can you take a look at https://github.com/snapcore/snapd/pull/10444 ? | 07:27 |
mardy | mborzecki: +1! | 07:44 |
mborzecki | thx | 07:44 |
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:11 |
pedronis | pstolowski: I reviewed https://github.com/snapcore/snapd/pull/10408 , thanks for the changes | 08:23 |
pstolowski | pedronis: thanks! | 08:25 |
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:33 |
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:34 |
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:35 |
pedronis | mardy: sorry, let me think a second, there might something easier that can we do | 08:39 |
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:40 |
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:45 |
pedronis | mardy: just make a snap/system_usernames.go instead | 08:52 |
=== pedronis_ is now known as pedronis | ||
mardy | pedronis: thanks :-) | 09:07 |
mborzecki | anyone remembers where was the spread binary that supported `-workers <n>` ? | 10:01 |
mborzecki | store seems to be a bit unhappy today | 10:14 |
mborzecki | seeing `error: cannot refresh "lxd": unexpectedly empty response from the server (try again later)` | 10:15 |
mborzecki | ehh a bit of a bummer with current tags | 10:38 |
zyga-mbp | haha | 10:39 |
zyga-mbp | it was the one from cachio | 10:39 |
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:40 |
pedronis | pstolowski: I put now https://github.com/snapcore/snapd/pull/7700 into my review queue | 10:44 |
pstolowski | ty | 10:45 |
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:48 |
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? | 10:50 |
zyga-mbp | mardy looking | 11:04 |
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:06 |
mardy | zyga-mbp: thanks, I'll retry it later then | 11:18 |
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:19 |
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:37 |
pedronis | mardy: you need to involve the fake store I fear to test this, otherwise you'll have no snap-ids | 11:38 |
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:39 |
pedronis | mardy: devicestate.CanManageRefreshes is similar in the sense it has a check that depends on the snap having a snap-id | 11:41 |
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:42 |
pedronis | mborzecki: I made some high-level comments in https://github.com/snapcore/snapd/pull/10436 | 11:43 |
mborzecki | thanks | 11:43 |
pedronis | mborzecki: added another one | 11:49 |
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:17 |
pstolowski | pedronis: dang, you are right, runHook will call our handler but then error the task | 12:21 |
pstolowski | pedronis: ok as a followup? | 12:22 |
pedronis | pstolowski: yes, but we need to decide a bit what to do there | 12:23 |
pedronis | pstolowski: I think we have very few hooks (handlers) fwiw that have non-trivial Error behavior | 12:26 |
pstolowski | pedronis: maybe even none? | 12:30 |
pedronis | pstolowski: healthstate I think does | 12:30 |
pstolowski | indeed | 12:31 |
pedronis | pstolowski: should we chat quickly about this now or after the standup? | 12:40 |
pstolowski | pedronis: sorry, was a bit distracted, maybe after standup then? | 12:58 |
pedronis | pstolowski: yes | 13:01 |
pstolowski | mardy: i added a general comment https://github.com/snapcore/snapd/pull/10438/files#r657180723, hope it makes sense | 14:52 |
mardy | pstolowski: 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!