[06:18] <zyga> mvo: good morning
[06:18] <zyga> fun test failure
[06:18] <zyga> no  void  directory on core18 https://www.irccloud.com/pastebin/hjyPHGW5/
[06:18] <mvo> zyga: uh, fun indeed
[06:18] <mvo> zyga: and easy to fix
[06:18] <mvo> zyga: thanks for finding this!
[06:19] <zyga> mvo: so
[06:19] <zyga> mvo: I have a problem
[06:19] <zyga> mvo: how  do we  fix this?
[06:19] <zyga> by hacking core18/
[06:19] <zyga> mvo: can we create a package that is designed for core18-like systems?
[06:19] <zyga> mvo: that has the right  startup core and mounting points
[06:20] <zyga> mvo: I would rather not have this conversation in core20
[06:20] <mvo> zyga: I would add it to the build tree of UC18, this will become the build tree of UC20
[06:20] <zyga> mvo: if we make the right package it will be simply inistalled in core18 systems and that will be enough
[06:21] <zyga> mvo: add it how? I'm arguing that it should be a standalone package built out of  snapd source package, that is then installed in core18 system
[06:22] <mvo> zyga: yeah, it could be snap-skeleton-dirs or something but I'm not sure this is more robust, we might also forgot add things there instead of in the main snapd package just like we could forgot adding things to the UC18/uc20 snaps. or do you have something different in midn?
[06:22] <mvo> mind
[06:22] <zyga> mvo: we can very well  forget but if we fix it the fix goes to core16, core18 and core20
[06:22] <zyga> mvo: in one patch
[06:22] <zyga> mvo: and it would be in the snapd repo, not somewhere else
[06:24] <mvo> zyga: how would that look like in pratcise? what would the package be called and what would the dependencies be?
[06:24] <zyga> mvo: it  would not depend on anything but it  would conflict with snapd.deb
[06:24] <zyga> mvo: the name is irrelevant, maybe  snapd-bootstrap-and-skeleton?
[06:25] <zyga> mvo: it would ship /var/lib/snapd structure, the /bin/snap symlink, the /usr/lib/snapd mount point and perhaps your bootstrap script
[06:25] <zyga> mvo: (anything appropriate)
[06:26] <mvo> zyga: it would be separate from the main package snapd which (afaics) has the same risk of getting our of getting out of sync. maybe we pull something from snapd git in to create the dirs/perms and use exactly this in the main snapd package build(s)?
[06:28] <zyga> mvo: hmm?
[06:29] <zyga> mvo: the snapd source package would have snapd.deb  binary package and snapd-skeleton.deb binary (arch all) package
[06:29] <zyga> mvo: is that sufficient or do I misundertand your worry?
[06:29] <zyga> *misunderstand
[06:29]  * zyga preps breakfast for kids, responses may be slower
[06:31] <mvo> zyga: snapd/snapd-skeleton may get out of sync if we are not careful
[06:32] <mvo> zyga: because we add a new dir (or something) to snapd but forget to add it to snapd-skeleton
[06:32] <mvo> zyga: so we need something that ensures that this can't happen, one way would be to use a makefile/script/whatever as part of the snapd package build and use exactly the same script as part of the uc18/uc20 build
[06:35] <mvo> zyga: alternatively we could create some of these missing dirs ourselfs maybe? /var/lib/snapd is fully writable
[06:35] <mvo> zyga: (create them if missing)
[06:35] <mvo> zyga: I actually like this as it means its easier if people try to run snapd from a snapd git checkout etc (slightly easier I admit :)
[06:38] <mborzecki> morning
[06:40] <mborzecki> wow, some prs are actually green
[06:41] <zyga> hmmm
[06:41] <zyga> I'm not sure
[06:41] <mborzecki> zyga: hey
[06:41] <zyga> this shoud run before  snap
[06:42] <zyga> before snapd
[06:44] <mvo> hey mborzecki
[06:44] <mborzecki> mvo: hey
[07:00] <mborzecki> zyga: idk if you noticed, but there's quite a few directories missing under /var/lib/snapd between core and core18
[07:06] <mvo> mborzecki: yeah, zyga and I discussed ways to fix this, zyga had some ideas how to make sure we have the layout inside our tree somehow so that its used in uc18/uc20 and we are exploring which one is the best
[07:14] <zyga> funny that we noticed just now
[07:14] <mborzecki> in most cases those are created on demand
[07:15] <mvo> yeah, I think on demand-creation is a nice way out of this
[07:15] <mvo> and it has the advantage of making it easy to run from e.g. checkouts
[07:16] <mvo> but *shrug* there are downsides too
[07:16] <zyga> mvo: I don't disagree in principle but not everything can be created on demand
[07:16] <mborzecki> iirc void has some special permissions too, so this would have to be accounted for when we create it on the fly
[07:19] <zyga> mborzecki: can you do a 2nd review on https://github.com/snapcore/snapd/pull/6645 please?
[07:19] <mborzecki> haha looking at it just now :)
[07:19] <zyga> thank you :)
[07:19] <zyga> I want the rush of adrenalin from landing a PR
[07:20] <mborzecki> still, suprising usb.ids doesn't list oneplus
[07:20] <zyga> maybe it's a fake request?
[07:20] <mborzecki> zyga: no, i found some pages describing the process of getting oneplus working on linux and they list the same vendor id
[07:21] <zyga> aha, that's a good find
[07:21]  * zyga reviews https://github.com/snapcore/snapd/pull/6634
[07:23] <mvo> mborzecki: silly question - what is needed for 6606 ? a second review or is there more ?
[07:24] <mborzecki> mvo: 2nd review
[07:26] <mborzecki> zyga: added some links under 6645 in case you're curious, there's even a patch from lineageos
[07:26] <zyga> mborzecki: looking
[07:27] <mvo> mborzecki: thanks, maybe pstolowski can do a second review on 6606, it looks pretty much ready to me
[07:31] <mvo> mborzecki: added my .0002¢ I think its fine but would feel better if pstolowski  could do  a final pass on 6606
[07:31] <mborzecki> mvo: thanks!
[07:47] <zyga> brb, coffee
[07:48] <zyga> https://github.com/snapcore/snapd/pull/6597 <- 2nd review needed in case someone wants to look
[07:48] <mvo> just a quick service announcement - we have an updated systemd in the edge core with a race fix, if anyone notices anything strange in the tests related to systemd please shout
[07:48] <mvo> zyga: I can look
[07:50] <zyga> mvo: thank you
[07:50] <zyga> mvo: alternatively look at https://github.com/snapcore/snapd/pull/6621/files
[07:50] <zyga> it's much smaller
[07:50] <zyga> and more on the critical path
[07:50] <zyga> mvo: ack, about systemd, thank you for that!
[07:50] <mvo> zyga: ha! thanks, I take 6621 I think :)
[07:51] <mvo> zyga: the other one is *big* it seem
[07:51] <mvo> zyga: hopefully both, but for that I need more tea
[07:51] <zyga> yeah, this one is very small, just tests and a few lines of new code
[07:59] <mvo> pedronis: do you want to review 6630 again (snap warnings yamlish) or can I merge? john addressed your review points
[08:03] <pstolowski> morning
[08:05] <mvo> pstolowski: good morning!
[08:05] <mborzecki> pstolowski: heya
[08:35]  * dot-tobias says hi
[08:36] <pedronis> mvo: hi, please merge
[08:38] <mvo> pedronis: ta
[09:09] <abeato> ppisati, morning!
[09:09] <abeato> ppisati, which is the apparmor status for 4.9 kernels? Is there any patch needed to run ubuntu core/snapd with it?
[09:11] <ppisati> abeato: not that i'm aware
[09:12] <zyga>  mvo I will merge 6605 after adding the extra comment
[09:12] <ppisati> abeato: *none, that i'm aware of
[09:12] <zyga> mvo: just wrapping up a small new branch you asked for
[09:12] <zyga> mvo: thank you for the review!
[09:14] <abeato> ppisati, ok, thanks
[09:16] <pstolowski> pedronis: i've debugged the ssh.disabled issue
[09:18] <mvo> zyga: ta
[09:23] <pstolowski> pedronis: we don't support dotted notation at the nested level of the defaults definition, we expand dotted path at the root level only, e.g. that would work (works in a unit test i made):
[09:23] <pstolowski> defaults:
[09:23] <pstolowski>   test-snap-ididididididididididid:
[09:23] <pstolowski>       service.ssh.disabled: true
[09:24] <pedronis> pstolowski: it sounds like we should error or do something though ?
[09:24] <pedronis> we don't support dots in name in principle?
[09:24] <pedronis> pstolowski: where is ther relevant code?
[09:24] <pstolowski> pedronis: and consequently, with the config that Sergio created you end up with unexpanded key in the config: map[string]interface {}{"ssh.disabled":true}
[09:25] <pedronis> yea, that looks wrong
[09:26] <pstolowski> pedronis: also, this works fine in unit test:
[09:26] <pstolowski> defaults:
[09:26] <pstolowski>   test-snap-ididididididididididid:
[09:26] <pstolowski>       service:
[09:26] <pstolowski>           ssh:
[09:26] <pstolowski>               disabled: true
[09:27] <pedronis> that I know
[09:27] <pstolowski> pedronis: we do support dots in name, but it needs to start at the root level. you cannot define map and then have dotted path deep inside
[09:27] <pedronis> pstolowski: I understand, you said it already
[09:28] <pstolowski> and yes we should error out i suppose
[09:28] <pedronis> yes,  "ssh.disabled" should't be a valid key
[09:28] <pedronis> we should not get there
[09:28] <pstolowski> let me see if i can find where it happens exactly in the code
[09:34] <zyga> mvo: https://github.com/snapcore/snapd/pull/6650 :-)
[09:36] <pedronis> zyga: shouldn't we try to land 6583 first?
[09:39] <pstolowski> pedronis: i think the important bit is the "for key, value := range patch" loop in func (h *configureHandler) Before(). we call tr.Set there for top-level patch entries, but inside transaction's Set we only ParseKey once (for that top level)
[09:40] <pedronis> pstolowski: so we are missing a check for dots in key (where unexpected) somewhere?
[09:42] <zyga> pedronis: I will be working on that today
[09:42] <pedronis> zyga: you have 17 open PRs , that's not a feature :)
[09:43] <zyga> pedronis: I know but many of them are very close to landing
[09:43] <pedronis> zyga: then please try to land before opening new ones
[09:43] <pedronis> or I will start to close new ones
[09:44] <zyga> pedronis: with regards to landing: we need a decision on what to do here https://github.com/snapcore/snapd/pull/6642#discussion_r268780713
[09:46] <pedronis> zyga: I agree with jdstrand if there's a way to keep it working without security issues it's better, we have been bitten already by taking stuff away from people
[09:46] <zyga> pedronis: so moving to void if we cannot stay in the current directory due to permission checks?
[09:47] <pedronis> yes
[09:47] <zyga> ok, I'll make the change
[09:47] <zyga> thanks, can you add a comment saying that in the PR, it will be easier for jamie to see
[09:48] <pedronis> done
[09:48] <zyga> Thanks
[09:53] <pstolowski> pedronis: yes, you could say that. i think we could support that in PatchConfig, although the fix is not immediately obvious
[09:54] <pedronis> pstolowski: anyway is not quite a priority right now, but we should plan to do something at some point
[09:54] <pstolowski> pedronis: ack
[09:54] <pedronis> pstolowski: I'm also not sure if it's better to error or to make it work
[09:54] <pedronis> (but splitting on dot9
[09:54] <pedronis> (by splitting on dots)
[09:59] <pedronis> mvo: do we use the standup ?
[09:59] <zyga> pedronis: https://github.com/snapcore/snapd/pull/6649 is a low hanging fruit
[10:00] <pedronis> ok
[10:03] <pedronis> degville: do you know where the meeting is?
[10:03] <degville> pedronis: nope. standup location? mvo is running a little late.
[10:24] <zyga> https://forum.snapcraft.io/t/ubuntu-18-04-fresh-install-default-snaps-are-updated-but-runtime-isnt/10579 this is interesting in a bad way
[10:25] <mborzecki> damn, Failed for "golang.org/x/crypto/cast5" (failed to clone repo): exit status 128
[10:27] <mborzecki> shal we land https://github.com/snapcore/snapd/pull/6623 ?
[10:29] <pedronis> mborzecki: seems so
[10:29] <mborzecki> it's sufficiently ripe :)
[10:30] <pedronis> it has a jdstrand +1  and other reviews
[10:30] <pedronis> at some point it sounded it needed to touch snap-confine, but is not the case
[10:31] <mborzecki> landed
[10:34] <zyga> eh
[10:35] <zyga> my tests also failed
[10:35] <zyga> 50 minutes of wasted time
[10:35] <zyga> fatal: unable to access 'https://go.googlesource.com/crypto/': Failed to connect to 2607:f8b0:400c:c06::52: Network is unreachable
[10:35] <zyga> this is why we need spread --fail-fast
[10:35] <zyga> this just sucks
[11:11]  * Chipaca hates all of the interfaces-many- spread tests
[11:13] <pstolowski> zyga: +1
[11:16] <niemeyer> mup_: So.. what's up with you?
[11:24] <niemeyer> mup: How're you doing now?
[11:24] <mup> niemeyer: I apologize, but I'm pretty strict about only responding to known commands.
[11:51] <mborzecki> 2019-03-26 10:33:46 Cannot allocate google:debian-sid-64: cannot perform Google request: Get https://www.googleapis.com/compute/v1/projects/computeengine/zones: oauth2: cannot fetch token: Post https://accounts.google.com/o/oauth2/token: dial tcp <IP_ADDRESS>:443: i/o timeout
[11:51] <mborzecki> hmmm
[11:52] <mborzecki> gcp broke?
[12:05] <Chipaca> mborzecki: nevah!
[12:05] <Chipaca> also, spread takes too long, so: while true; do printf "\u$(printf "%x" $((0x2571 + RANDOM%2)))"; done
[12:07] <mborzecki> heh, and i got into a little mess with the 'gadget' package, some code is on review right now, and merging the branches breaks all of it :P
[12:08] <mborzecki> aand another job hit the time limit
[12:09] <mborzecki> it's quite intersting, so prepare fails on some nodes because golang.org is flaky and times out, then the nodes that actually made it execute all the tests and the job hits travis time limits
[12:14] <mborzecki> maybe it's because of https://twitter.com/MehreenKhn/status/1110509604176384000
[12:19]  * zyga goes to the doctor
[12:19] <zyga> ttyl
[12:22] <mup> PR snapcraft#2515 closed: build providers: support for provider setup <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2515>
[12:24] <Chipaca> mvo: question about the snapd snap when you have a bit
[12:25] <cachio> pstolowski, https://api.travis-ci.org/v3/job/511064184/log.txt
[12:25] <Chipaca> mvo: the question is whether building with snapcraft from the snap (instead of from the deb), and adding -b -Zgzip to the dpkg-buildpackage in the plugin, make sense to you
[12:25] <cachio> this is the log for the ssh.disable: true
[12:25] <Chipaca> mvo: the first is because otherwise it leaves behind a bunch of packages that break the invariant
[12:26] <Chipaca> mvo: the second is because 300s → 90s
[12:26] <Chipaca> mvo: (I could just change the first, the second is merely opportunistic)
[12:30] <pstolowski> cachio: i know, i debugged it in the morning and discussed with pedronis; we don't currently support dotted paths nested deep inside defaults; it would work only from top-level, e.g. service.ssh.disabled: true. we should at the very least provide a meaningful error message or make it smarter
[12:32] <mvo> Chipaca: +1 on the second, I'm not fully getting the first one, sorry (probably a bit slow today)
[12:34] <pstolowski> cachio: btw i think it found the cause of interfaces-daemon-notify failure in my nested vm PR; i think it fails because of "plan: n1-standard-2" (?!); i run this single test a couple of times today with and without this change (also with just master) and it seems to be it. if that plan is enabled, then "journalctl -u snap.test-snapd-daemon-notify.notify.service" that we execute at the end of the test doesn't return any
[12:34] <pstolowski> output for that service. i debugged this manually and when it fails, system log has the expected "Permission denied" errors in it, it's just not reported by journalctl for whatever reasons. systemd/journalctl versions appear to be the same both with and without that plan.
[12:35] <pstolowski> i've juse reverted this plan from https://github.com/snapcore/snapd/pull/6491 and it passed
[12:35] <mup> PR #6491: interfaces: hotplug nested vm test, updated serial-port interface <Hotplug 🔌> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6491>
[12:36] <pstolowski> pedronis: ^ is green
[12:36] <pedronis> cachio: I'll comment in the PR when I have a bit of time today
[12:36] <pedronis> (about ssh.disable)
[12:36] <Chipaca> mvo: only builds the binary deb, uses gzip to compress it (instead of also building source and using xz)
[12:36] <Chipaca> mvo: oh wait
[12:37] <Chipaca> mvo: the first one, changing deb for snap, because 'apt install snapcraft" pulls in e.g. python3-crypto, which autoremove can't
[12:37] <Chipaca> can't autoremove i mean
[12:37] <Chipaca> because things recommend it or maybe suggest it i guess
[12:37] <cachio> pstolowski, which is the PR?
[12:37] <mvo> Chipaca: +1
[12:37] <cachio> pedronis, thanks
[12:38] <mvo> Chipaca: oh, so "snap install snapcraft"?
[12:38] <Chipaca> mvo: yar
[12:38] <pstolowski> cachio: #6491
[12:38] <mvo> Chipaca: this is the change? if so, yes, lets do it
[12:38] <Chipaca> ok
[12:38] <mup> PR #6491: interfaces: hotplug nested vm test, updated serial-port interface <Hotplug 🔌> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6491>
[12:38] <mvo> Chipaca: thank you! I think htis is just old code from the time when we had no snap (ironically)
[12:38] <Chipaca> mvo: it'll be part of the invariant pr unless you'd rather it went in alone
[12:38] <mvo> Chipaca: and nice idea on the dpkg-buildpackage one
[12:38] <pstolowski> cachio: can you think on any reason why this more performant plan makes that test fail? any difference in the images?
[12:38] <mvo> Chipaca: fine for me
[12:38] <Chipaca> k
[12:38] <mvo> Chipaca: its not a big change :)
[12:38] <mvo> Chipaca: thank you!
[12:40] <cachio> pstolowski, did you change the plan?
[12:40] <cachio> I didn't see that
[12:58] <pstolowski> cachio: yes i reverted that change innym PR to make it pass
[12:58] <pstolowski> cachio: see last commit
[13:00] <pstolowski> cachio: just apply it on master and it should fail the interfaces-daemon-notify test
[13:00] <mup> PR snapd#6651 opened: tests: all the systems for google backend with 6 workers <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6651>
[13:01]  * zyga off to the doctor
[13:02]  * Chipaca off to have lunch
[13:02] <Chipaca> zyga: lunch > doctor
[13:02]  * Chipaca wins
[13:03] <cachio> pstolowski, mmmm, ok
[13:03] <cachio> pstolowski, we can't change the plan
[13:05] <zyga> What is the status of spread / cloud?
[13:05] <zyga> Did we run out of money?
[13:08] <pstolowski> cachio: hmm hey not? How Is It affecting 14.04?
[13:12] <mup> PR snapd#6606 closed: selinux, systemd: support mount contexts for snap images <SELinux> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6606>
[13:13] <mborzecki> zyga: do you want to take a look at https://github.com/snapcore/snapd/pull/6485 ?
[13:13] <mup> PR #6485: interfaces/seccomp: regenerate changed profiles only <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6485>
[13:29] <dot-tobias> jdstrand: Is there a way to force an NTP resync using the timezone-control DBus interface? https://www.freedesktop.org/wiki/Software/systemd/timedated/ indicates that SetNTP() method restarts systemd-timedated, but timedatectl still reports “NTP in sync: no”. Use case: Our device is offline until configured via an access point, so the device is booting without network. System does not seem to pick up the available network connection in the
[13:29] <dot-tobias>  minutes after its established.
[13:32] <zyga> back from doctor
[13:32] <zyga> bikes are amazing :)
[13:32] <zyga> mborzecki: sure, looking
[13:56]  * Chipaca ⇝ cuppa
[14:00] <mup> PR core#38 closed: Add another pi-config option <Created by sergey-borovkov> <https://github.com/snapcore/core/pull/38>
[14:00] <mup> PR core#83 closed: move most of the ubuntu-core config deb into the snap snap build <Created by mvo5> <https://github.com/snapcore/core/pull/83>
[14:01] <mup> PR core#38 opened: Add another pi-config option <Created by sergey-borovkov> <https://github.com/snapcore/core/pull/38>
[14:01] <mup> PR core#83 opened: move most of the ubuntu-core config deb into the snap snap build <Created by mvo5> <https://github.com/snapcore/core/pull/83>
[14:05] <jdstrand> dot-tobias: I'm not an expert on timedated. If I were to guess, I'd guess the clock is maybe too far off or that your device is unable to communicate with the configured server. you might also check your logs for policy violations, but it sounds like timedatectl is working ok and it is something else
[14:07] <dot-tobias> jdstrand: Yes, the clock was off > 12h because that's the time when the image was created. Didn't know that in order to sync the clock has to be near the actual date. Will look into workarounds how to get in re-sync.
[14:07] <dot-tobias> jdstrand: Thanks!
[14:08] <mup> PR snapd#6329 closed: cmd/snap-confine, packaging: support SELinux <SELinux> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6329>
[14:11] <jdstrand> dot-tobias: cool. ogra may have some thoughts on that as well
[14:14] <zyga> hey jdstrand :)
[14:14] <zyga> jdstrand: I'm glad you noticed "goto the void" :-)
[14:14] <zyga> initially it was an else statement but then it was too good to pass :-)
[14:17] <jdstrand> zyga: hey, yeah :)
[14:17] <zyga> :-)
[14:17] <zyga> jdstrand: we also managed to uncover that core18 is missing some directories in that test, it's better late than never
[14:18] <jdstrand> zyga: I saw that, wild :)
[14:21] <zyga> jdstrand: I implemented the next branch but I will defer until my set of open PRs shrinks sufficiently
[14:21] <zyga> jdstrand: btw, not sure if you saw https://github.com/snapcore/snapd/pull/6649
[14:21] <mup> PR #6649: snap: reject layouts to /lib/{firmware,modules} <Created by zyga> <https://github.com/snapcore/snapd/pull/6649>
[14:49] <mup> PR snapd#6652 opened: sanity: use proper SELinux mount context when sanity checking squashfs mounts <SELinux> <Simple 😃> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6652>
[14:50]  * zyga -> tea / coffee
[14:51] <mborzecki> super simple selinux PR ^^
[15:06] <ogra> zyga, mvo, do you gys know if anyone actually tested pi core18 images with wlan ? seems i dont get proper DNS resolution to finish console-conf here (network is up and i can ping the board but user creation times out with DNS issues finding the login.u.c api stuff)
[15:07] <ogra> this is with the stable released image...
[15:08] <mvo> ogra: I'm not sure we did, sorry. is this the dragonboard or pi3?
[15:08] <ogra> jdstrand, in case dot-tobias comes back while i'm not around ... https://github.com/ogra1/ntpcontrol
[15:08] <mvo> ogra: or both?
[15:08] <ogra> mvo, pi image on a pi 3 A+
[15:09] <ogra> core 16 works fine
[15:09] <ogra> (i resorted to that in the end)
[15:12] <mvo> ogra: we need to look into this, in a meeting now unfortunately
[15:13] <ogra> mvo, all fine, just wanted to check if it was tested before
[15:13] <ogra> i also resoted to core16 for the project i needed the board for ... but i have another A+ for any tests if required
[15:18] <mvo> ogra: added to test/debug this to our trello, thanks for reporting
[15:22] <mup> PR snapd#6649 closed: snap: reject layouts to /lib/{firmware,modules} <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6649>
[15:34]  * cachio lunch
[15:35] <cwayne> ogra: sorry if this message is doubled, my ircs been weird but we do test wlan on Pi's, but using nm
[15:35] <ogra> well, that wont help much if NM isnt preinstalled :P
[15:35] <ogra> (also ... does console-conf actually support NM ??)
[16:13] <niemeyer> cachio: spread#75 reviewed again
[16:13] <mup> PR spread#75: Make spread tests for spread project run on google backend <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/75>
[16:17] <mup> PR snapcraft#2516 opened: readme: add snap store badge <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2516>
[16:19] <zyga> s pedronis renamed https://github.com/snapcore/snapd/pull/6621
[16:19] <mup> PR #6621: overlord/snapstate: track time of postponed refreshes <Created by zyga> <https://github.com/snapcore/snapd/pull/6621>
[16:20] <zyga> pedronis: to "inhibit"
[16:20] <zyga> it is green so I'm very tempted to merge it :)
[16:20] <zyga> everyone: I need 2nd review for https://github.com/snapcore/snapd/pull/6597
[16:20] <mup> PR #6597: cmd/snap-update-ns: refactor of profile application (1/N) <Created by zyga> <https://github.com/snapcore/snapd/pull/6597>
[16:20] <pedronis> zyga: it doesn't have 2 +1
[16:20] <zyga> pedronis: I know :)
[16:20] <zyga> that's why I'm saying
[16:20] <zyga> if that was your only reservation I'd love to get a 2nd one
[16:21] <pedronis> zyga: I asked Chipaca to review it
[16:22] <zyga> great, I'll wait for the review then
[16:22] <pedronis> Chipaca: could you look at #6621
[16:22] <mup> PR #6621: overlord/snapstate: track time of postponed refreshes <Created by zyga> <https://github.com/snapcore/snapd/pull/6621>
[16:22] <pedronis> when you have time
[16:22] <Chipaca> yes
[16:40] <cachio> niemeyer, thanks
[16:41] <ijohnson> fyi to anybody using the go remote part, I just updated it in the wiki, so if anything breaks ping me
[16:41] <zyga> thank you ijohnson
[16:42] <Chipaca> huh
[16:43] <zyga> Chipaca: hmm?
[16:43] <Chipaca> zyga: what permissions should snap-confine have?
[16:43] <zyga> Chipaca: as in UNIX?
[16:43] <Chipaca> zyga: ye
[16:44] <zyga> for now u+rwXs,g+rXs,o:+rX
[16:44] <zyga> though I will drop the g+S part
[16:44] <zyga> why?
[16:45] <Chipaca> zyga: because it starts at 06755, as you say
[16:45] <Chipaca> zyga: but snap-confine-from-core sets it to 04755
[16:45] <zyga> wait, what?
[16:45] <zyga> why?
[16:45]  * zyga hugs Chipaca for fixing the tests
[16:46] <Chipaca> restore: |
[16:46] <Chipaca>     echo "Restoring host snap-confine"
[16:46] <Chipaca>     chmod 4755 /usr/lib/snapd/snap-confine
[16:46] <Chipaca> zyga: ^
[16:46] <zyga> la la la
[16:47] <zyga> man
[16:47] <zyga> ...
[16:47] <zyga> what just happened
[16:47] <zyga> I spent 10 minutes debugging
[16:47] <zyga> why snapd doesn't build
[16:47] <zyga> on ... macos
[16:47] <Chipaca> zyga: snap does :-) but not snapd :-(
[16:47] <zyga> wrong terminal tab on a common shared source
[16:47] <zyga> lol
[16:47]  * Chipaca is still happy that snap does :-)
[16:47] <zyga> I aborted a rebase in progress because I got fed up with this and wanted to think why everything is broken
[16:48] <Chipaca> zyga: what was it telling you wasn't implemented?
[16:48] <zyga>  /Users/zyga/go/src/github.com/snapcore/snapd/interfaces/mount/backend.go:97:84: undefined: osutil.MountEntry
[16:49] <zyga> I totally ignored the path
[16:50]  * Chipaca nods
[16:51] <mborzecki> fwiw there was this crazy linux distro that didn't use the usual / layout, they could have had /User too
[16:51] <Chipaca> it's just noise
[16:51] <Chipaca> noise][1: no offense
[16:51] <Chipaca> :-)
[16:51] <zyga> mborzecki: yeah, I used it once
[16:51] <zyga> it had /Users and /Applications like on macos
[16:53] <pedronis> mborzecki: I did a pass over the gadget validation PR
[16:54] <mborzecki> pedronis: thanks, sorry for it being so long
[16:55] <pedronis> mborzecki: we'll need another layer of validation that looks at actual content for sizes as well?
[16:55] <mborzecki> pedronis: yes, that will need to access actual snap data
[16:56] <pedronis> ok
[17:02] <pedronis> zyga: are you waiting for review from me on anything but #6642 atm ?
[17:02] <mup> PR #6642: cmd/snap-confine: prevent cwd restore permission bypass <Created by zyga> <https://github.com/snapcore/snapd/pull/6642>
[17:02] <zyga> checking
[17:03] <zyga> we need  to look at https://github.com/snapcore/snapd/pull/6583
[17:03] <mup> PR #6583: cmd/snap-confine: move ubuntu-core fallback checks <Created by zyga> <https://github.com/snapcore/snapd/pull/6583>
[17:03] <zyga> but perhaps tomorrow morning
[17:03] <pedronis> I'm not entirely happy with the code I saw the there the last time I looked
[17:03] <pedronis> we can chat tomorrow
[17:03] <zyga> today I want to finish mending existing branches that I know  what needs to  happen wiht
[17:03] <zyga> ok
[17:04] <pedronis> I'll try to still look at 6642 tonight
[17:05] <zyga> thank you!
[17:05] <zyga> it should go green once I fix core18 later  today
[17:05] <mup> PR snapcraft#2516 closed: readme: add snap store badge <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2516>
[17:13] <mup> PR snapd#6485 closed: interfaces/seccomp: regenerate changed profiles only <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6485>
[17:13] <Chipaca> zyga: how is having RefreshInhibitedTime be zero on success useful?
[17:13] <zyga> it is  useful because that is correct
[17:13] <zyga> we had refreshed
[17:13] <zyga> whee
[17:13] <zyga> kill the  inhibit time value, if we had  any set
[17:14] <zyga> next time when refresh is inhibited we can  check for non-zero inhibit time
[17:14] <zyga> and if the time of  initial inhibition goes beyond certain duration
[17:14] <Chipaca> hold on
[17:14] <zyga> we can take action (kill apps etc)
[17:15] <Chipaca> if the refresh wasn't inhibited, the task will be Done
[17:15] <zyga> and?
[17:17] <Chipaca> zyga: currently, if a refresh is inhibited, the task doesn't reach Done, right?
[17:17] <zyga> it's not implemented in this  branch but something along those lines, correct
[17:17] <zyga> we don't get to create those tasks in some cases
[17:17] <zyga> things just bail out early
[17:18] <zyga> sometimes we will create tasks and bail out late
[17:18] <Chipaca> zyga: where do you track the inhibited time when you don't create the task?
[17:18] <zyga> it's in the snap state all along
[17:18] <zyga> it's not tracked in tasks
[17:18] <zyga> the one in tasks is just for undo handler
[17:22] <zyga> Chipaca: does it make sense?
[17:22] <Chipaca> zyga: if you don't create the task, won't the refresh by retried much much later than intended?
[17:23] <Chipaca> zyga: why would you not download the snap i mean
[17:23] <mup> PR snapd#6646 closed: interfaces/adb-support: account for hubs on sysfs path <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6646>
[17:23] <Chipaca> zyga: but yes, the code makes sense now, I somehow thought you were keeping it in the task (d'oh)
[17:23] <Chipaca> I didn't realie this was super-task
[17:23] <zyga> Chipaca: initially not creating the tasks is the easy way out
[17:24] <Chipaca> I'm not sure how that works though (hence my last question)
[17:24] <zyga> Chipaca: I will download the revision as an update to that flow
[17:24] <zyga> Chipaca: but I want to have the basic version in first
[17:24] <zyga> Chipaca: I will use the soft check (in another PR) to determine if we create tasks or not
[17:25] <zyga> Chipaca: then the hard check after stopping services to see if we abort or not
[17:25] <Chipaca> after stopping services?
[17:25] <zyga> Chipaca: then if soft  or hard checks go over $MAGIC_DURATION  we carry on regardless
[17:25] <zyga> Chipaca: ah, sorry, before
[17:25] <Chipaca> ah
[17:25] <zyga> (in this model it is before)
[17:25] <zyga> no, no wait
[17:25] <zyga> after
[17:26] <zyga> yeah, I was right the  first time
[17:26] <zyga> we do a hard check after stopping services because this is the moment we can migrate the data safely
[17:26] <zyga> though I may be tired and talk nonsense, that part is not proposed yet and I haven't ran it in weeks
[17:29] <pstolowski> pedronis: zyga can we land https://github.com/snapcore/snapd/pull/6491 ?
[17:29] <mup> PR #6491: interfaces: hotplug nested vm test, updated serial-port interface <Hotplug 🔌> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6491>
[17:30] <Chipaca> buuuuh, github is being silly
[17:31] <zyga> pstolowski: looking
[17:31] <Chipaca> zyga: I tried to +1 with a "blocked" tag, but github isn't letting me
[17:32] <Chipaca> zyga: so I set it to 'changes requested', because i do request changes in fact, but my bigger concern is not landing it before the pr that uses it is ready to land
[17:32] <zyga> Chipaca: why is that a concern?
[17:32] <Chipaca> zyga: because without that, it's just cruft :)
[17:32] <zyga> I wanted to land this deliberately before so that in case someone turns the feature off the data in  state is  correct
[17:32] <Chipaca> it does nothing useful on its own
[17:33] <Chipaca> ?
[17:33] <Chipaca> wat
[17:33] <zyga> when the rest of the  code lands that new  chunk will be behind a feature  flag
[17:33] <zyga> but that's fine, it can land later
[17:34] <Chipaca> zyga: what in the state would be incorrect if somebody enabled and then disabled this?
[17:34] <zyga> Chipaca: if this ships but the  rest is in edge, going to  stable will give you correct state
[17:35] <zyga> Chipaca: though I agree it's a bit silly and not strictly tied to the follow up branch being present
[17:35] <Chipaca> zyga: if that is a concern, you need an epoch
[17:35] <zyga> Chipaca: I will work on the  follow up as soon as the hard/soft branch lands
[17:35] <zyga> Chipaca: no, it's not a concern,  just pedantry
[17:36] <Chipaca> zyga: what happens if they enable this, get the flag in state, go back to stable, time passes, and they get the feature?
[17:37] <Chipaca> zyga: and they had something inhibited when they went back to stable
[17:37] <Chipaca> what happens then
[17:37] <zyga> if the  inhibit time is  zero
[17:37] <zyga> nothing
[17:37] <zyga> if they had some value they may get a forced refresh if it goes over 90  days or something similar
[17:38] <Chipaca> doesn't sound too bad. OTOH you could zero them in a subpatch
[17:38] <Chipaca> ?
[17:39] <zyga> yeah
[17:39] <zyga> once we enable the feature perhaps/
[17:46] <zyga> Chipaca: fixed, thank you
[17:46] <zyga> pstolowski|afk: I will review it again, sorry for the lagg
[18:04] <mup> PR snapd#6653 opened: tests: try to be a little bit invariant <Created by chipaca> <https://github.com/snapcore/snapd/pull/6653>
[19:17] <om26er> Is this channel still connected to Slack ? if so, what technology is used for the bridge ?
[19:25] <mup> PR pc-amd64-gadget#10 closed: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/10>
[19:25] <mup> PR pc-amd64-gadget#11 closed: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/11>
[19:26] <mup> PR pc-amd64-gadget#10 opened: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/10>
[19:26] <mup> PR pc-amd64-gadget#11 opened: Add mmx64.efi (MokManager) to support mokutil <Created by tsunghanliu> <https://github.com/snapcore/pc-amd64-gadget/pull/11>
[19:27] <zyga> om26er: I don't  believe it  ever was
[19:27] <om26er> zyga, ok, I think I confused it with another channel
[20:22] <zyga> jdstrand: please enqueue https://github.com/snapcore/snapd/pull/6605 if you have some time, it has 2 +1s and just needs your review
[20:22] <mup> PR #6605: cmd/libsnap,osutil: fix parsing of mountinfo <Created by zyga> <https://github.com/snapcore/snapd/pull/6605>
[21:27] <juliank> hmm, how does snapctl work?
[21:27] <juliank> sudo snapctl stop radicale
[21:27] <juliank> error: error running snapctl: cannot stop without a context
[21:27] <juliank> $ sudo snapctl services
[21:27] <juliank> error: error running snapctl: cannot query services without a context
[21:27] <ijohnson> juliank: are you using snapctl outside of a snap?
[21:28] <juliank> ah, it's just for use by snaps?
[21:28] <ijohnson> it's only usable inside a snap
[21:28] <juliank> it should maybe say so :)
[21:28] <ijohnson> outside of a snap you should just use `snap`
[21:28] <ijohnson> i.e. snap stop radicale
[21:30] <juliank> So I did sudo snap stop --disable radicale.daemon
[21:30] <juliank> will that persist after an upgrade of the snap?
[21:30] <ijohnson> it is supposed to but there's currently a bug where refreshes always re-enable services
[21:30] <juliank> Because with my local testing snap, if I do snap install my.snap --dangerous, it would override it :)
[21:31] <juliank> hmm
[21:31] <juliank> that's suboptimal
[21:31] <ijohnson> see https://bugs.launchpad.net/snapd/+bug/1818306
[21:31] <mup> Bug #1818306: disabled daemons are started after refresh <snapd:New> <https://launchpad.net/bugs/1818306>
[21:32] <juliank> I'm thinking about running radicale for caldav/carddav on my server, so I sanpped the current version - but I don't wanna run the daemon as root (even in strict confinement), so I wanted to disable the daemon and just run it as a user program (well, from a systemd service)
[21:33] <juliank> I do want to ship the daemon because duh, that's more useful for the store :)
[21:33] <ijohnson> how would you run it as a user program from a systemd service?
[21:33] <juliank> I don't know, I have not tried yet
[21:34] <juliank> Exec=/snap/bin/radicale I guess?
[21:34] <juliank> with User=radicale
[21:34] <ijohnson> hmm I guess that would probably work
[21:34] <ijohnson> there is upcoming snapd work that would allow the daemon to drop from root to a specific "daemon" user instead, but that's not done yet
[21:35] <juliank> it would confine it to access to ~radicale/snap/common instead of /var/snap/radicale/common, but that's fine
[21:35] <juliank> (it's a strict snap :D)
[21:36] <juliank> ijohnson: I read about that
[21:37] <juliank> (the only plug it needs is network-bind :D)
[21:37]  * cachio afk
[22:30] <jdstrand> zyga: ack


[00:03] <wallyworld> thumper: shiteveld won't accept by latest diff for the rsyslog upgrader. can you take a peek in lp and see if you are happy? i made a small tweak and responded to comments
[00:20] <thumper> ok, will look
[00:25] <thumper> wallyworld: for your Replace file thing, why not use Stat to get the existing premission, and only use 0644 if you can't get it?
[00:26] <wallyworld> thumper: cause we want the new perms to overwite any exisitng ones
[00:26] <stokachu> hey, i sent an email to the mailing list but just throwing this here as well, anyone know the latest version of the websocket specification juju api server supports?
[00:26] <thumper> wallyworld: but you aren't letting the user specify the permissions
[00:26] <thumper> you are explicit in 0644
[00:26] <thumper> which may not be right
[00:26] <thumper> stokachu: I don't sorry
[00:27] <wallyworld> oh shit, i should have that passed in
[00:27] <thumper> wallyworld: if we want to overwrite perms, allow the caller to pass them in
[00:27] <thumper> :)
[00:27] <stokachu> how do i tell which version of the websocket library is used from go.net/websocket?
[00:27] <wallyworld> that's what my brain was thinking but my fingers didn;t type that
[00:27] <thumper> wallyworld: I get that all the time too
[00:28] <wallyworld> stokachu: i'm not sure. rogpeppe is the best person to ask
[00:28] <wallyworld> but he's past EOD now
[00:28] <stokachu> ok
[00:29] <wallyworld> i'll poke him later to make sure he responds
[00:29] <stokachu> wallyworld: thanks a bunch
[00:29] <wallyworld> np
[00:50] <wallyworld> thumper: fixed
[00:50] <thumper> kk
[01:04] <thumper> axw: no os.Groups though ...
[01:04] <thumper> also easier to mock out in the tests with the command :-)
[01:04] <axw> thumper: there's Gid?
[01:04] <thumper> but thanks for the pointer
[01:04] <thumper> not on a os.User
[01:04] <thumper> oh
[01:04] <axw> yes there is
[01:04] <thumper> so there is
[01:04] <axw> :)
[01:05] <axw> anyway, not a problem, just wanted to make sure you knew
[01:05] <thumper> cheers
[01:05] <thumper> os.User.Gid is a string and os.Chown needs an int
[01:05] <thumper> lovely library coherency there
[01:07] <thumper> axw: I'm happy with https://code.launchpad.net/~wallyworld/juju-core/rsyslogconf-upgrader-118/+merge/206090, you happy?
[01:08] <wallyworld> yep, for this iteration
[01:08] <axw> thumper: yeah I was gonna LGTM
[01:08] <thumper> cool
[01:08] <axw> FTR, +1 for a sub package for utils
[01:09] <wallyworld> axw: i reverted to using TempDir cause then the file perms work put nicer - TempFile creates the file with 600 and you need more code to change the perms
[01:09] <wallyworld> i think that's why it was done that way originally
[01:09] <axw> ok no worries
[01:46] <thumper> axw: got a minute?
[01:46] <thumper> bugger
[01:46] <thumper> completely missed the standup
[01:47] <thumper> what a slacker
[01:47] <thumper> oops, and sorry
[01:47] <wallyworld> f*cking bot
[01:47]  * wallyworld stabs it to death
[01:49] <axw> thumper: yes
[01:49] <axw> what's up?
[01:50] <thumper> axw: was wondering about bootstrapping prcess with maas
[01:50] <thumper> and how the sync bootstrap handles the networking bits in it
[01:50] <thumper> particulary how we currently restart networking
[01:50] <axw> un moment, gotta refresh memory on maas
[01:51] <thumper> I'm likely to change the "edit /etc/network/interfaces, restart networking"
[01:52] <thumper> to be "ifdown eth0, edit /etc/network/interfaces, ifup br0"
[01:52] <thumper> which I have been told will work, but I need to test it
[01:52] <thumper> these are all different lines in the cloud init scripts section
[01:52] <thumper> wondering how the sync bootstrap handles this...
[01:53] <thumper> wondering idly if this relates to : bug 1233751
[01:53] <_mup_> Bug #1233751: MAAS environment bootstrapped, but not really. <bootstrap> <destroy-environment> <jenv> <maas-provider> <theme-oil> <juju-core:Triaged> <https://launchpad.net/bugs/1233751>
[01:53] <axw> thumper: I think cloud-init will run that bit. still trying to get my head around it...
[01:54] <axw> if that's the case it should work no problems
[01:55] <axw> thumper: yep, maas's StartInstance does that networking bit, so cloud-init will do the networking change
[01:55] <axw> so it won't break the ssh connection
[01:55]  * axw looks at the bug
[01:55] <thumper> how do we handle the bootstrapping?
[01:56] <thumper> axw: doesn't bootstrap call start instance?
[01:56] <axw> thumper: I suspect that bug is just cos the bootstrap-state file got left behind on failed bootstrap
[01:56] <axw> yes it does, but there's two parts to synchronous bootstrap
[01:56] <axw> part 1: run cloud-init with a very basic cloud-config
[01:57] <axw> for maas, there's an additional bit of modifying networking
[01:57] <axw> part 2: wait for the machine to come up, ssh in and do the rest
[01:57] <thumper> ok, cool, so the hack networking is all part 1?
[01:57] <axw> and "do the rest" is provider-independent
[01:57] <axw> yep
[01:57] <thumper> axw: is the bug then fixed?
[01:57] <axw> thumper: dunno sorry
[01:57] <thumper> kk, so I'll manually test the ifdown/ifup story locally on my precise maching :)
[01:57] <thumper> machine
[01:58] <axw> doesn't look related to synchronous bootstrap
[01:58] <axw> pre-bootstrap setup I think
[01:58] <thumper> ok, but didn't you just fix a bug that deleted the .jenv if the bootstrap fails?
[01:58] <axw> oh that's a different file
[01:58] <axw> I'm talking about how we leave a file in cloud storage
[01:58] <thumper> ah... :(
[02:09] <axw> thumper: actually, I'm not sure now. you may get the same error for existing .jenv
[02:09] <axw> I would need a NUC or something to test on ;)
[02:12] <bigjools> curtis has one IIRC
[02:21] <thumper> bigjools: can I test something on your maas?
[02:21] <bigjools> maaaaaybe
[02:22] <bigjools> it means I have to go and turn it on
[02:31] <bigjools> thumper: it's on, can you get in?  I can't remember if my router config still works for you
[02:31] <thumper> um...
[02:31] <bigjools> since you're sshing into a network two NATs deep :)
[02:31] <thumper> what was the ip address and port again?
[02:31]  * bigjools rolls eyes
[02:43] <thumper> bigjools: I have auth to go buy some NUCs for maas testing locally
[02:43] <thumper> I just need to get my a into g and get some
[02:43] <bigjools> thumper: expensed?
[02:43] <thumper> aye
[02:43] <bigjools> suhweet
[02:43] <bigjools> how much in NZ?
[02:44] <thumper> not sure
[02:44] <thumper> have to go price some up
[04:10]  * axw goes offline for a while to run tests without a network
[06:10] <axw> wallyworld: FYI I have looked at your upgrader CL. It looks good, but I don't think I understand everything well enough to +1 at the moment
[06:11] <wallyworld> axw: that's ok, it was meant for william to look at anyway :-)
[06:11] <axw> ok, cool
[06:12] <wallyworld> thanks for looking though
[06:12] <axw> nps
[06:12] <wallyworld> it achieves the same result as a previous version, but with a different approach
[06:15] <axw> yup, so I see. I understand the idea and see how it works, just not fully aware of how the watcher stuff works for example
[06:16] <wallyworld> it's a bit involved
[07:25] <wallyworld> fwereade: no urgency, take 2 - https://codereview.appspot.com/63730043/
[07:26] <wallyworld> the main tests i copied across from the other branch and made then pass with the new approach
[08:05] <fwereade> wallyworld, nice, thanks
[08:05] <fwereade> wallyworld, hey, I was wondering
[08:05] <fwereade> wallyworld, using ssl-hostname-verification for metadata-needs-signing seems a little strange to me
[08:07] <wallyworld> which bit of code?
[08:07] <wallyworld> i'll look
[08:07] <fwereade> wallyworld, err let me hunt again, I came upon something that looked like it was doing that while trying to figure out some of the joyent provider tests last night
[08:08] <fwereade> wallyworld, if that doesn't ring a bell I probably just misunderstood it
[08:08] <wallyworld> i can't recall offhand either
[08:09] <wallyworld> gotta go for dinner, back later
[08:09] <fwereade> wallyworld, ah it's ok I completely misunderstood it
[08:10] <wallyworld> o :-)
[08:10] <wallyworld> ok
[08:10] <fwereade> wallyworld, it really is about verifying the hostnames
[08:10] <fwereade> wallyworld, enjoy your dinner
[08:10] <wallyworld> will do
[08:41] <fwereade> I've got a trivial intermittent failure if anyone's got a moment: https://codereview.appspot.com/63710044
[08:54] <fwereade> rogpeppe, 479-desired-peer-group appears to be LGTMed from a month ago but still unmerged
[08:54] <rogpeppe> fwereade: oh twats
[08:55] <rogpeppe> fwereade: it's always that problem when you approve something then switch context back to what you were doing, and forget to check that it's actually merged
[08:55] <fwereade> mgz, can we land https://codereview.appspot.com/49050044/ ?
[08:55] <fwereade> rogpeppe, yeah, I have a couple of those myself :/
[08:56] <fwereade> TheMue, is 058-debug-log-api abandoned in favour of a different approach?
[08:57] <fwereade> rogpeppe, also 487-remove-job-managestate
[08:58] <rogpeppe> fwereade: oh, that's worse
[08:58] <rogpeppe> fwereade: bugger
[08:58] <rogpeppe> fwereade: looks like today is the day for bountiful conflicts
[08:59] <fwereade> TheMue, I'm rejecting 058 and WIPing 060
[08:59]  * fwereade wishes rogpeppe luck
[09:03] <TheMue> fwereade: I'm currently testing the 060 live, it is the solution. Only coded testing is missing. But it works for all providers (even local) and with older releases (even here now local).
[09:04] <TheMue> fwereade: The 060 contains both, the backend and the cmd changes.
[09:05] <fwereade> TheMue, excellent, thanks
[09:20] <rogpeppe> wallyworld: this is from a suggestion i made in a review - i didn't want to badger you about it, but i think it's worth doing: https://codereview.appspot.com/63530048
[09:24] <wallyworld> rogpeppe: we prefer interfaces
[09:24] <rogpeppe> wallyworld: why?
[09:24] <wallyworld> all the usual reasons
[09:24] <rogpeppe> wallyworld: when we've just got a bag of data, there is no reason to use an interface
[09:25] <rogpeppe> wallyworld: it just obfuscates the code for no reason
[09:25] <rogpeppe> fwereade: i'd appreciate your thoughts on this
[09:25] <wallyworld> i'm tired tonight so may not be in the best frame of mind for a coherent discussion
[09:25] <rogpeppe> wallyworld: i appreciate that interfaces are useful when you might want to mock different behaviour
[09:26] <rogpeppe> wallyworld: but in this case there is no behaviour to mock - the data structure *is* the interface
[09:26] <wallyworld> now yes
[09:26] <wallyworld> bit the design is evolving
[09:26] <wallyworld> but
[09:27] <wallyworld> the same pattern has been used for agent conf
[09:27] <rogpeppe> wallyworld: if we need extra behaviour, that will be easy enough to retro-fit. let's not overengineer in advance - this is not Java
[09:27] <wallyworld> huh?
[09:27] <rogpeppe> wallyworld: agent conf does actually have some behaviour behind it
[09:27] <wallyworld> as will the upgrade stuff most likely
[09:28] <rogpeppe> wallyworld: (it encapsulates the way to read different versions of the file)
[09:28] <rogpeppe> wallyworld: i don't see that - this is a description of the upgrade steps - that's fundamentally data AFAICS. it's also held inside the binary, not in an external file
[09:29] <wallyworld> it is data for sure. but the design is still evolving
[09:30] <rogpeppe> wallyworld: sure, so evolve the data
[09:30] <rogpeppe> wallyworld: having the extra layer of indirection doesn't help that (actually, it makes it harder)
[09:31] <rogpeppe> wallyworld: "KISS" :-)
[09:31] <wallyworld> sure. there's arguments either way
[09:33] <rogpeppe> fwereade: can we please go the simpler route to start with?
[09:33] <rogpeppe> fwereade: if you find that interfaces become useful here, i will eat my humble pie
[09:34] <wallyworld> to me interfaces are simpler because they abstract behaviour and mocks can easily be constrcuted for only some of the methods
[09:34] <wallyworld> and it's generally just plan good programming practice
[09:34] <fwereade> rogpeppe, wallyworld: IMO our reliance on concrete types has fucked the codebase pretty hard
[09:34] <wallyworld> our use of structs everywhere in juju core kinda sucks
[09:34] <rogpeppe> fwereade: i don't think this case is analagous to that
[09:35] <fwereade> rogpeppe, wallyworld: once you turn out to need an interface it's a *massive* hassle to fix the concrete types
[09:35] <wallyworld> +1
[09:36] <rogpeppe> fwereade: so do you really think we should never use structs? even when we're just holding bags of data?
[09:36] <fwereade> rogpeppe, I want enough seams in the codebase to allow for easy testing of small units, without backing everything with real implementations
[09:36] <wallyworld> bags of data now for some things, but the design is evolving
[09:36] <wallyworld> +1
[09:37] <rogpeppe> fwereade: there is no "implementation" here - it's just a description of what to do
[09:37] <rogpeppe> fwereade: the "implementation" is in the Run functions which are already fully mockable
[09:37] <rogpeppe> fwereade: i can't see what use it is to "mock" a function that just returns a description and does nothing more.
[09:38] <fwereade> rogpeppe, I'm curious as to what you perceive the benefits of dropping the interface to be
[09:38] <rogpeppe> fwereade: it 60 lines less code. i consider that a good reason in itself.
[09:38] <rogpeppe> s/it/it's/
[09:39] <wallyworld> rogpeppe: you are the one who likes lots of code :-)
[09:39] <fwereade> rogpeppe, and I'm not so sure that we'll never see behaviour associated with things like Targets
[09:39] <wallyworld> all those duplicated "contains" functions etc
[09:39] <fwereade> rogpeppe, by pickinginterfaces from the get-go we can put extensions to behaviour in the right place when the time comes
[09:40] <wallyworld> they gie us more flexibility to evolve a design
[09:40] <wallyworld> give
[09:40] <rogpeppe> fwereade: it is also much more immediately obvious what the code is doing - if i see a field access, i don't have to look at what some arbitrary code might be doing
[09:40] <fwereade> rogpeppe, if we don't, then the "simple" approach becomes to tweak the code that uses those types
[09:40] <fwereade> rogpeppe, and behaviour gets smeared out
[09:40] <fwereade> rogpeppe, and things just get worse and worse
[09:41] <fwereade> rogpeppe, because even if we *know* it's the time to switch to interfaces
[09:41] <fwereade> rogpeppe, the hassle and cost of doing so is generally overwhelming
[09:41] <fwereade> rogpeppe, you don;t have to anyway
[09:41] <rogpeppe> fwereade: i really don't believe that could be the case here - this is limited-scope code
[09:41] <fwereade> rogpeppe, the upgrade step knows where it should be applied
[09:42] <fwereade> rogpeppe, as a client it's not your responsibility to figure it out, and it's none of your business how it's figured out *anyway*
[09:42] <rogpeppe> fwereade: if we want behaviour on Targets, that's trivial to add.
[09:42] <fwereade> rogpeppe, and so it begins
[09:42] <fwereade> rogpeppe, just a little bit of coupling can't hurt
[09:42] <rogpeppe> fwereade: it's not coupling
[09:43] <rogpeppe> fwereade: it's evolving the schema
[09:43] <fwereade> rogpeppe, and forcing the clients to care about that evolution
[09:43] <rogpeppe> fwereade: what clients?
[09:43] <rogpeppe> fwereade: this is essentially an internal-use data structure only
[09:44] <fwereade> rogpeppe, that doesn't mean there aren't clients
[09:44] <fwereade> rogpeppe, the code that uses it, and the code that tests it, are the minimum set of clients for just about any construct
[09:44] <rogpeppe> fwereade: so by "client" you mean the package itself?
[09:45] <fwereade> rogpeppe, yes
[09:45] <fwereade> rogpeppe, programming is *all about* managing interfaces to impose simplicity on complexity
[09:46] <fwereade> rogpeppe, package boundaries are way too coarse to be useful
[09:46] <rogpeppe> fwereade: sure, but an "interface" can just as well be a concrete data structure there
[09:47] <rogpeppe> fwereade: i've switched back and forth between interfaces and concrete types many times. i much prefer starting with the simple, and then graduating to the more elaborate when it proves necessary.
[09:47] <fwereade> rogpeppe, I do not agree with that approach
[09:47] <fwereade> rogpeppe, I gave it a fair shake
[09:47] <rogpeppe> fwereade: if you can't agree that a simple concrete type is appropriate for representing a simple concrete bunch of data, i give up
[09:48] <fwereade> rogpeppe, and I'm confident that most of the current complexity of juju is a result of an adherence to flawed notions of simplicity
[09:48] <fwereade> rogpeppe, it'd be a different story in python
[09:48] <fwereade> rogpeppe, because you can use a descriptor in place of a field when it becomes necessary
[09:49] <fwereade> rogpeppe, but with go you *invest* in the struct approach
[09:49] <fwereade> rogpeppe, it's an expected value calculation
[09:49] <rogpeppe> fwereade: you really think that most of the complexity of juju is from using structs?
[09:50] <fwereade> rogpeppe, most of the bits that really hurt and continue to do so, like jujud, are the way they are because we have no seams by which to pick them apart
[09:50] <fwereade> rogpeppe, the state/environ horrors are because we have a state struct
[09:50] <rogpeppe> fwereade: i think most of the pain there is in the tests, not the code
[09:51] <fwereade> rogpeppe, the inability to test state quickly and easily are because of all the structs in mgo which makes it completely unmockable
[09:51] <rogpeppe> fwereade: because we had a prior aversion to mocking anything
[09:51] <rogpeppe> fwereade: huh? we could mock mgo easily
[09:51] <fwereade> rogpeppe, the awful awful uniter stucture is the same story -- there's nothing *that bad* about it in theory, but in practice you can't isolate any of it and have to take the whol lot as an indivisible unit
[09:51] <rogpeppe> fwereade: well, except that we'd need to mock mongodb semantics
[09:52] <rogpeppe> fwereade: again, i really think that's because of our prior aversion to mocking anything
[09:52] <fwereade> rogpeppe, maybe I'm being dense: how would you mock anything without interfaces?
[09:52] <rogpeppe> fwereade: there are various places in the code where we *do* manage to mock pieces of the State to good effect
[09:53] <rogpeppe> fwereade: you don't need to *export* the type as an interface
[09:53] <rogpeppe> fwereade: you just need to define clients in terms of an interface, and then use some trivial bridge code (or nothing) to bridge the gap
[09:54] <rogpeppe> fwereade: it's all about ad-hoc interfaces
[09:54] <fwereade> rogpeppe, isn't that what wallyworld is doing? it's just that you consider the clients in this case as to be too trivial to bother with
[09:54]  * wallyworld apologises - too tired to contribute tonight
[09:55] <fwereade> wallyworld, don't worry, not calling you into the conversation :)
[09:55] <rogpeppe> fwereade: if there was any behaviour to mock, i might agree. but there isn't, and i can see no particular prospect of it - it's really a data-driven algorithm
[09:56] <rogpeppe> fwereade: for a static table of data, a static table of data seems like the right representation to me
[09:56] <fwereade> rogpeppe, I think what he'd doing is relying on instinct for bits-likely-to-change, and inserting seams where they're likely to be useful
[09:56] <fwereade> rogpeppe, apart from the Run bits
[09:57] <fwereade> rogpeppe, they're types with data *and* behaviour
[09:57] <rogpeppe> fwereade: tbh what i think what's happening is just use-interfaces-by-rote
[09:57] <rogpeppe> fwereade: the behaviour is independent of the data
[09:58] <fwereade> rogpeppe, your refactoring still puts them in the same type; I think that's an indication that it's not *quite* as cut and dried as yu say
[09:58] <fwereade> rogpeppe, note: this is not a request for a more extreme refactoring ;p
[09:59] <rogpeppe> fwereade: yes, that type composes both the metadata and some behaviour together
[10:00] <fwereade> rogpeppe, did I say the thing about expected value?
[10:00] <fwereade> rogpeppe, not sure I did
[10:00] <rogpeppe> fwereade: not sure you did
[10:01] <fwereade> rogpeppe, picking an interface has a small cost now and in future; picking a struct has a lower cost now, and a chance of a much higher cost in future
[10:01] <fwereade> rogpeppe, the choice comes down to a judgment call
[10:01] <fwereade> rogpeppe, people can legitimately disagree
[10:01] <rogpeppe> fwereade: you really think that there's a high cost from moving to use an interface? when there are no external clients?
[10:02] <rogpeppe> fwereade: i have not found that
[10:02] <rogpeppe> fwereade: i like the code to be as understandable as possible
[10:02] <fwereade> rogpeppe, yes, I really do
[10:03] <fwereade> rogpeppe, IMO/E interfaces are at least as understandable as structs
[10:03] <rogpeppe> fwereade: so you think that changing the code base to use, for example, all the State types as interfaces, would be hard?
[10:03] <fwereade> rogpeppe, I'm trying t obe generous in acknowledging that sometimes they're a little cheaper
[10:04] <rogpeppe> fwereade: interfaces are less understandable because it's not obvious what's going to happen when they're called
[10:04] <fwereade> rogpeppe, yes, it would be a giant hassle and a massive diff
[10:04] <fwereade> rogpeppe, because it's *not the client's business*
[10:04] <rogpeppe> fwereade: it would be a big diff certainly (*state.State -> state.State) but actually it would be easy
[10:04] <fwereade> rogpeppe, and all the other types?
[10:04] <rogpeppe> fwereade: in this case we *are* the client!
[10:05] <rogpeppe> fwereade: sure, it wouldn't be a problem
[10:05] <rogpeppe> fwereade: i could do it this afternoon if we wanted
[10:05] <fwereade> rogpeppe, there's no use in having a State interface if Machine returns a *Machine, you have to return Machine as well
[10:05] <rogpeppe> fwereade: sure, that's why i said "all the State types"
[10:06] <rogpeppe> fwereade: the only hassle would be maintaining the interface types independently from the struct definitions - the interfaces would be enormous.
[10:06] <rogpeppe> s/struct definitions/method definitions/
[10:06] <fwereade> rogpeppe, right, so you *can't* usefully use an ad-hoc interface because you have to construct an actual *Machine
[10:07] <rogpeppe> fwereade: you can, but it's a bit more work
[10:07] <rogpeppe> fwereade: (the address updater uses that technique for testing)
[10:07] <fwereade> rogpeppe, it's an *enormous* amount of work to construct one of those that behaves in a specifically tuned way
[10:07] <fwereade> rogpeppe, and even *that* is hamstrung because it depends on concrete mgo types
[10:08] <fwereade> rogpeppe, it's *really nice* to be able to test state clients against tuned mocks, rather than building up loads of state every time
[10:09] <rogpeppe> fwereade: honestly, it's not *that* much hassle - here's the relevant code the instancepoller tests: http://paste.ubuntu.com/6930374/fwereade: actually the best value would come not from mocking *
[10:09] <rogpeppe> oops
[10:09] <rogpeppe> fwereade: honestly, it's not *that* much hassle - here's the relevant code the instancepoller tests: http://paste.ubuntu.com/6930374
[10:10] <rogpeppe> fwereade: actually the best value would come not from mocking *state.State, but the various other types (*Machine &c)
[10:11] <fwereade> rogpeppe, multiply that work by N tests for N state clients and it becomes a bit unwieldy
[10:11] <fwereade> rogpeppe, it's the same simple-code-repeated-everywhere problem
[10:12] <fwereade> rogpeppe, that I'm not sure yu see as an actual problem
[10:13] <rogpeppe> fwereade: so if State was an interface, how would that help matters here?
[10:13] <rogpeppe> fwereade: you'd still need to mock out the actual behaviour
[10:13] <rogpeppe> fwereade: so you'd probably end up with an enormous "one-size-fits-all" state mocker, which would be its own source of considerable complexity
[10:14] <rogpeppe> fwereade: i've thought sometimes that it might be nice just to mock out the mgo interface, and write some in-memory mongodb interpretation code
[10:15] <rogpeppe> fwereade: that would mean that you could base all of the State code in memory, so it would be loads faster
[10:15] <rogpeppe> fwereade: but... you'd still want to be certain that the mock mgo semantics mirrored the actual mongo semantics
[10:15] <fwereade> rogpeppe, sure *that* would be nice, but again I don't see how you could actually do that
[10:15] <fwereade> rogpeppe, well, actually, *not* necessarily
[10:16] <rogpeppe> fwereade: again, it would be straightforward to do
[10:16] <fwereade> rogpeppe, mocking lets you exhaustively test the operation of a client given a range of locally defined behaviours
[10:16] <fwereade> rogpeppe, imagining that the only purpose of a mock is to perfectly simulate a real system is to somewhat miss the point
[10:17] <fwereade> rogpeppe, what-happens-if-X-bizarre-situation becomes *actually* testable, and cheaply
[10:17] <rogpeppe> fwereade: the problem is that the integration tests don't necessarily test all the behaviours that are being tested in the unit tests
[10:18] <rogpeppe> fwereade: ideally, you'd be able to test against both the mock (for quick tests) and the real thing (for slower but more complete) tests
[10:19] <fwereade> rogpeppe, integration tests as tracer bullets, and exhaustively paranoid unit tests with mocks, are O(N) work; exhaustively paranoid integration tests are O(N^layers-used-directly-or-indirecttly)
[10:19] <jamespage> is it really right that 1.16.6 added errgo?
[10:19] <fwereade> rogpeppe, oh yes, more direct switch of subject
[10:19] <rogpeppe> jamespage: definitely not
[10:19] <fwereade> rogpeppe, if we're depending on it can we please put it under github/juju
[10:19] <jamespage> rogpeppe, well its in the release tarball...
[10:19]  * jamespage sighs
[10:20] <rogpeppe> jamespage: i know why
[10:20] <jamespage> I'll ping sinzui later
[10:20] <rogpeppe> jamespage: it's because of the way that the tarball is built
[10:20] <rogpeppe> jamespage: it's not actually *used* by juju in 1.16.6
[10:21] <rogpeppe> jamespage: but the tarball is build by doing "go get .../juju" then updating to the correct revision, then updating the deps
[10:21] <rogpeppe> jamespage: but updating the deps does not remove unused deps
[10:21] <jamespage> rogpeppe, oh crap - so it will pull in all trunk deps but not drop ones used in stable
[10:21] <jamespage> not used that would be
[10:21] <rogpeppe> jamespage: yes
[10:22] <jamespage> that's not helpful
[10:22] <rogpeppe> jamespage: that's my understanding
[10:22] <jamespage> rogpeppe, lemme nag sinzui later
[10:22] <jamespage> I was going to upload this for SRU but stuff like this will make the SRU team nervous
[10:22] <rogpeppe> jamespage: what's the actual problem with having unused deps?
[10:23] <rogpeppe> fwereade: this'll be thumper's errgo, FWIW
[10:23] <fwereade> rogpeppe, sure, the request still stands
[10:24] <rogpeppe> fwereade: ok
[10:24] <rogpeppe> fwereade: you don't think that github.com/errgo/errors is a reasonable path for it?
[10:24] <rogpeppe> fwereade: it would be considerably more likely for 3rd party code to use it there, i think, and that would be a Good Thing
[10:27] <fwereade> rogpeppe, I'm not sure that's the case -- and I really don't want to further grow our dependencies on external code
[10:28] <fwereade> rogpeppe, it'll be a pretty fundamental dependency
[10:28] <fwereade> rogpeppe, and if it's good and stable I don;t think being under an organisation named for a (hopefully) successful and popular open-source package will do it any harm
[10:29] <rogpeppe> fwereade: perhaps so.
[10:30] <rogpeppe> fwereade: i'll probably put it at the above path too, and make sure the two are compatible
[10:30] <fwereade> rogpeppe, it remains a distinct package with its own identity but we get to move towards drawing our critical dependencies into a sphere over which we have control
[10:30] <fwereade> rogpeppe, that's perfectly fine
[10:32] <fwereade> rogpeppe, jamespage: btw, AIUI, we *could* chck out juju, and use deps.tsv to pull in the necessary dependencies on their own, and then not run the risk of pulling in other random unpinned stuff, right? we just don't do that yet
[10:32] <rogpeppe> fwereade: the problem with that is my fault
[10:33] <fwereade> rogpeppe, jamespage: but I don't see any reason not to?
[10:33] <rogpeppe> fwereade: which is that godeps doesn't currently have a way to pull deps - it just updates existing deps
[10:33] <fwereade> rogpeppe, yeah, I can see that the pulling-deps bit is a hassle
[10:33] <rogpeppe> fwereade: i should just do that - i've just been lazy so far
[10:33] <fwereade> rogpeppe, that would be awesome
[10:34] <fwereade> rogpeppe, btw can I steal 30s of attention for a review of https://codereview.appspot.com/63710044/ please?
[10:34] <rogpeppe> fwereade: sure
[10:35] <rogpeppe> fwereade: what's the deal with the changes to UniterAPI.getUnit and getService?
[10:36] <fwereade> rogpeppe, oh balls
[10:36] <fwereade> rogpeppe, wrong base
[10:36] <fwereade> rogpeppe, it's https://codereview.appspot.com/57710043/
[10:37] <fwereade> rogpeppe, the reason is that there's no guarantee that we're actually passing unit or service tags there
[10:38] <rogpeppe> fwereade: yeah, i figured that.
[10:38] <rogpeppe> fwereade: it could still use FindEntity and just check the type-assertion result, but the ParseTag approach is cheaper and probably better, yeah
[10:48] <rogpeppe> fwereade: i don't quite understand how that CL restores the ability of the uniter to read removed units
[10:48] <fwereade> rogpeppe, getUnit fails if the unit doesn't exist
[10:48] <fwereade> rogpeppe, pulling out the service name still works so long as the service is there
[10:49] <fwereade> rogpeppe, and it will be so long as the relation using it is
[10:49] <rogpeppe> fwereade: but didn't we just call getUnit to get the RelationUnit ?
[10:49] <rogpeppe> fwereade: that's operated on by checkRemoteUnit
[10:50] <fwereade> rogpeppe, the RelationUnitis for LocalUnit
[10:51] <rogpeppe> fwereade: ah ha!
[10:51] <rogpeppe> fwereade: ok, makes sense.
[10:53] <rogpeppe> fwereade: LGTM with a suggestion for a comment
[10:54] <fwereade> rogpeppe, SGTM, would you LGTM the other one for form's sake too please?
[10:54] <rogpeppe> fwereade: am looking currentyl
[10:54] <natefinch> morning all. Seems like I missed the standup?
[10:57] <fwereade> natefinch, er, I didn't spot the standup reminder :/
[10:57] <fwereade> rogpeppe, natefinch: shall we do it quickly then?
[10:58] <rogpeppe> fwereade: i didn't either
[10:58] <rogpeppe> fwereade, natefinch: let's
[10:59] <rogpeppe> mgz: standup?
[10:59] <rogpeppe> mgz: (better late than never...)
[11:21] <TheMue> could somebody please give me a little hit at the back of my head! somehow I'm in vi mode and trying to operate my sublime editor with vi commands. *aaaargh*
[11:59] <rogpeppe> anyone see launchpad.net down ?
[12:01] <natefinch> rogpeppe: fine for me
[12:02] <rogpeppe> natefinch: ah, it's started working again
[12:02] <natefinch> rogpeppe: you're welcome ;)
[12:02] <rogpeppe> natefinch: although it doesn't seem to be registering my branch pushes
[12:02] <rogpeppe> i'm trying to approve this: https://code.launchpad.net/~rogpeppe/juju-core/487-remove-job-managestate/+merge/202916
[12:03] <rogpeppe> but it keeps insisting that it's on revno 2250, not the latest revno 2252
[12:04] <rogpeppe> and as expected the bot is saying "There are additional revisions which have not been approved in review"
[12:04] <rogpeppe> how frustrating
[12:04] <natefinch> rogpeppe: yeah weird
[12:05] <rogpeppe> natefinch: if you do, "bzr revno -r lp:~rogpeppe/juju-core/487-remove-job-managestate", it prints 2252, right?
[12:05] <natefinch> rogpeppe: it prints ??? as the response, which is less than encouraging
[12:06] <rogpeppe> natefinch: ah, that might be just bzr - can you pull the branch?
[12:07] <natefinch> rogpeppe: yep, and once that's done it says 2252
[12:08] <rogpeppe> natefinch: right, thanks
[12:08] <rogpeppe> natefinch: so *something* is up with lp
[12:08] <rogpeppe> mgz: any idea what could cause this?
[12:09] <rogpeppe> cd ..
[12:12] <rogpeppe> hmm, interesting bzr push error:
[12:12] <rogpeppe> bzr: ERROR: Server sent an unexpected error: ('error', 'xmlrpclib.ProtocolError', '<ProtocolError for xmlrpc.lp.internal:8097/codehosting: -1 >')
[12:14] <natefinch> rogpeppe: have you tried rebooting? :D
[12:14] <mgz> hm, nothing particular comes to mind
[12:14] <rogpeppe> natefinch: it doesn't seem like a local problem to me
[12:14] <rogpeppe> another error: ConnectionReset reading response for 'BzrDir.open_2.1', retrying
[12:15] <rogpeppe> but then the retry succeeded
[12:15] <natefinch> rogpeppe: that's the joke.  But yes, aggravating.
[12:15] <mgz> may be a remote issue
[12:15] <rogpeppe> mgz: so no idea of how i might be able to prod lp into recognising that a branch has been updated?
[12:16] <rogpeppe> ha, "Sorry, there was a problem connecting to the Launchpad server."
[12:17] <rogpeppe> so it's not just one branch
[12:17] <rogpeppe> natefinch: what happens if you go to https://code.launchpad.net/~rogpeppe/juju-core/479-desired-peer-group/+merge/201245 now?
[12:19] <mgz> rogpeppe: still getting an error now?
[12:19] <natefinch> rogpeppe: that's a different MP, is that the right link?
[12:20] <rogpeppe> natefinch: yeah, i was getting an error on that one too
[12:20] <rogpeppe> mgz: i just succeeded with that one. let me try the first one again
[12:22] <rogpeppe> mgz: getting an error with that one still; will keep reloading the page
[12:23] <rogpeppe> ah, finally!
[12:23] <rogpeppe> approved at the right version
[12:31] <natefinch> rogpeppe: what's the proper way to join path strings in a path list? our code does a lot of path+":"+newpath  but that'll fail on windows.... but os.PathListSeparator is a rune, not a string, which makes it a pain in the ass
[12:33] <rogpeppe> natefinch: i saw a proposal ages ago for a filepath.SplitList, but i think it foundered
[12:33] <rogpeppe> natefinch: s/SplitList/JoinList
[12:34] <rogpeppe> natefinch: we could always add it to utils
[12:35] <natefinch> rogpeppe: ok, yeah, I had hoped I just missed it somewhere
[12:35] <rogpeppe> natefinch: func JoinList(paths ...string) {return strings.Join(paths, string(os.PathListSeparator))}
[12:36] <rogpeppe> natefinch: actually, given that it's in utils, it should probably be JoinPathList
[12:36] <natefinch> rogpeppe: ahh, didn't realize you could cast a rune to a string.
[12:36] <rogpeppe> natefinch: you should read the spec again :-)
[12:37] <natefinch> yeah, it's been a while.  Just haven't dealth with runes much
[12:53] <rogpeppe> structural regexp line noise of the moment: ,x/^=.*\n([^=].*\n)+/y/^=.*\n(---.*\n)?(\+\+\+.*\n)?/x/(^[+\-].*\n)+/g/(errors|errgo)\.Cause/=
[12:55]  * mgz hits rogpeppe
[12:56]  * rogpeppe protests: "but it's doing useful stuff for me!"
[12:58] <rogpeppe> mgz: perhaps you've got a better idea actually.
[12:59] <rogpeppe> mgz: i've got an enormous branch that i want to cherry pick certain bits from automatically
[12:59] <mgz> na, not really, rewrites like this are always just finding the right pain balance of automation and manual work
[12:59] <rogpeppe> mgz: (or at any rate with a minimum of effort)
[13:00] <rogpeppe> mgz: do you know of a tool that lets me just click on the bits i want to choose?
[13:00] <mgz> hm, well, with an existing branch, depending on how the bits are layed out, you may find a merge and a proper three-way editor faster
[13:00] <natefinch> now you have two problems etc
[13:01] <natefinch> yeah, a proper merge tool makes it a lot easier
[13:02] <mgz> natefinch: do you have a particular one you tend to use?
[13:02] <mgz> I struggle along with vimdiff when I need it generally
[13:03] <natefinch> mgz: beyond compare is awesome
[13:04] <natefinch> mgz meld is supposed to be pretty awesome but I haven't used it
[13:19] <rogpeppe> natefinch: i'm just trying meld, but it's not easy to see how it supports my use case
[13:21] <natefinch> rogpeppe: you should be able to diff a clean branch vs. your fixed branch then just do a search for .Cause and copy over the changes that you want to keep
[13:22] <rogpeppe> natefinch: thanks, that sounds plausible, i'll try that
[13:23] <natefinch> rogpeppe: yeah, since you said there weren't that many spots in which you use cause, it's probably faster and easier than trying to get exactly the right regex to work.
[13:23] <rogpeppe> natefinch: yeah, and the regex isn't sufficient anyway, because there are other parts related to Cause changes that don't mention Cause
[13:24] <natefinch> rogpeppe: ahh yeah. Sometimes there's no replacement for the human brain
[13:57] <natefinch> arg... I kinda wish we hadn't used testing/testbase in utils, because then it means we can't use utils in testing/testbase :/
[13:58] <niemeyer> Hey all
[13:58] <niemeyer> fwereade: Is the juju-dist bucket still being used?
[14:00] <niemeyer> Hmm, looks like it is
[14:08] <TheMue> *: did anybody had a "cannot upload charm: Post https://localhost:8900/charms?series=quantal: local error: record overflow" in client_test.go TestAddLocalCharm()?
[14:09] <fwereade> niemeyer, it's still being uploaded to and while it's probably not being used *much* we did come across some guys with a very old environment recently
[14:09] <fwereade> niemeyer, where there's one there may be more
[14:09] <TheMue> fwereade, rogpeppe: do you know the test failure mentioned above?
[14:10] <rogpeppe> TheMue: i haven't seen that
[14:10] <TheMue> rogpeppe: thx, strange
[14:11] <niemeyer> fwereade: Hmmm
[14:11] <niemeyer> fwereade: Do we have any releases in Ubuntu using it?
[14:12] <fwereade> TheMue, hmm, haven't seen that
[14:14] <fwereade> niemeyer, I don't *think* so -- 1.14 is the latest that depends on it iirc -- but they need it to upgrade to 1.16
[14:15] <niemeyer> fwereade: Ah, still recent then, cool
[14:15] <TheMue> rogpeppe, fwereade: running the test solo it passes, running the suite it fails ???
[14:15] <rogpeppe> TheMue: it fails reliably?
[14:16] <TheMue> rogpeppe: seems so, had been able to repeat it multiple times
[14:16] <TheMue> rogpeppe: the "record overflow" is no text in our code
[14:17] <rogpeppe> TheMue: interesting.
[14:17] <rogpeppe> TheMue: what does the test output look like?
[14:18] <TheMue> rogpeppe: http://paste.ubuntu.com/6931384/
[14:21] <TheMue> rogpeppe: and the piece of code is http://paste.ubuntu.com/6931388/
[14:22] <rogpeppe> TheMue: interesting - i'm not sure that could ever have worked
[14:22] <TheMue> rogpeppe: hehe
[14:23] <TheMue> rogpeppe: but as I said, running the test function as the only one it works
[14:23] <rogpeppe> oh, i see; it sets the server root
[14:23] <rogpeppe> TheMue: i suspect you've got two tests colliding
[14:23] <rogpeppe> TheMue: that test should definitely not be listening on a fixed port
[14:24] <TheMue> rogpeppe: sounds logical
[14:24] <rogpeppe> TheMue: if you change that code to listen on a newly chosen port instead, does it still fail?
[14:25] <rogpeppe> TheMue: ah, there's another problem with that code too - it may not have actually started listening when you add the local charm
[14:25] <TheMue> rogpeppe: changed to 8901, still fails
[14:26] <rogpeppe> TheMue: could you change it to listen on a newly chosen port (use net.Listen("tcp", ":0"))
[14:26] <rogpeppe> ?
[14:28] <TheMue> rogpeppe: have to check how I would have to change the code, but now I've got my standup
[14:29] <rogpeppe> TheMue: the same way that all the other test http listeners do it
[14:29] <TheMue> rogpeppe: will look there
[15:00] <TheMue> rogpeppe: changed it and it runs on the same failure
[15:00] <rogpeppe> TheMue: what does the code look like now?
[15:02] <TheMue> rogpeppe: paste.ubuntu.com/6931586/ - and it still passes when called alone
[15:03] <rogpeppe> TheMue: hmm
[15:05] <rogpeppe> TheMue: one possible thing to try: in the juju-core root directory, run "godeps -u dependencies.tsv"
[15:07] <TheMue> rogpeppe: could do so, but could you explain the reason for this command before?
[15:07] <rogpeppe> TheMue: it updates the package dependencies to be as expected
[15:07] <rogpeppe> TheMue: also, what version of Go are you using?
[15:08] <TheMue> rogpeppe: 1.1.1
[15:08] <TheMue> rogpeppe: should update ;)
[15:09] <rogpeppe> TheMue: yes, you must do that
[15:09] <rogpeppe> TheMue: you should be using 1.2
[15:09] <rogpeppe> TheMue: if you're not up to date, you'll probably need to iterate - when godeps complains about a package (because the required revision isn't available), run go get -u <pkgname>
[15:09] <TheMue> rogpeppe: ok, will quickly change
[15:09] <sinzui> hi rogpeppe , natefinch : Do either of you have a few minutes to review a backport for 1.16 that lets QA run unit-tests? https://codereview.appspot.com/64000043
[15:10] <rogpeppe> sinzui: looking
[15:11] <rogpeppe> sinzui: LGTM
[15:11] <sinzui> thank you rogpeppe
[15:16]  * rogpeppe goes for lunch
[15:22]  * natefinch goes to blow some snow
[15:23] <TheMue> rogpeppe: after update and rebuild of all pkgs still the same
[16:28] <rogpeppe> fwereade: https://github.com/juju/errgo
[16:28] <rogpeppe> fwereade: (README thanks to github.com/robertkrimen/godocdown )
[16:39] <fwereade> rogpeppe, awesome, tyvm
[17:54] <sinzui> any ideas about what I can do to make gobot test my branch again? https://code.launchpad.net/~sinzui/juju-core/run-unit-test-on-trusty/+merge/206503
[17:59] <rogpeppe> sinzui: sometimes it gets stuck
[18:02] <rogpeppe> sinzui: it should be working again now
[18:02] <sinzui> plars in #juju reports this error on hp-cloud. The config works for 1.16.3, but not 1.17.2
[18:02] <sinzui> ERROR bootstrap failed: cannot start bootstrap instance: index file has no data for cloud {region-a.geo-1 https://region-a.geo-1.identity.hpcloudsvc.com:35357/v2.0/} not found
[18:02] <rogpeppe> sinzui: BTW i updated your commit message - it's really nice if the commit message is exactly the same as is in the codereview description, including the codereview link
[18:02] <sinzui> I cannot reproduce the issue
[18:03] <sinzui> noted rogpeppe
[18:03] <rogpeppe> sinzui: i'm away for the weekend now
[18:03] <rogpeppe> happy weekends all
[18:03] <sinzui> me too