| davecheney | waigani_: menn0 thumper https://github.com/juju/juju/pull/548 | 00:19 |
|---|---|---|
| davecheney | here is an easy one to warm up with this morning | 00:19 |
| hazmat | anyone know instance distributor provider code? | 00:35 |
| hazmat | just curious how it knows service name | 00:35 |
| menn0 | thumper: got a sec? | 01:28 |
| thumper | menn0: aye | 01:28 |
| menn0 | thumper: hangout? | 01:28 |
| thumper | sure | 01:28 |
| menn0 | thumper: calling you now | 01:29 |
| menn0 | thumper: https://plus.google.com/hangouts/_/g4pl2c7bq3jqz2jy3rsjdtvakea?hl=en-GB | 01:29 |
| menn0 | thumper: grrr | 01:29 |
| menn0 | let's use the standup hangout | 01:30 |
| thumper | ok | 01:30 |
| davecheney | wow, my branch from a month ago merged mostly cleanly with master | 01:38 |
| davecheney | not bad | 01:38 |
| thumper | \o/ | 01:46 |
| axw | can I please get a review on https://github.com/juju/juju/pull/552 to unblock CI? | 02:14 |
| axw | wallyworld_: ^^ very small one | 02:16 |
| wallyworld_ | sure | 02:16 |
| davecheney | OOPS: 34 passed, 6 FAILED | 03:15 |
| davecheney | --- FAIL: Test (18.44s) | 03:15 |
| davecheney | FAIL | 03:15 |
| davecheney | exit status 1 | 03:15 |
| davecheney | FAIL github.com/juju/juju/state/apiserver/provisioner 18.541s | 03:15 |
| davecheney | sigh | 03:15 |
| davecheney | well, at least it's only 3 of the apiserver packages | 03:16 |
| davecheney | which is better than before | 03:16 |
| davecheney | but still a lot of tests to fix | 03:16 |
| thumper | waigani: I'm about to drop maia down at hockey | 03:17 |
| thumper | waigani: how about we hangout when I get back and talk about the testing ? | 03:17 |
| waigani | thumper: that would be great, thanks | 03:17 |
| davecheney | ... obtained params.ErrorResults = params.ErrorResults{Results:[]params.ErrorResult{params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:¶ms.Error{"not found", "machine 42 not found"}}, params.ErrorResult{Error:¶ms.Error{"not found", "unit \"foo/0\" not found"}}, params.ErrorResult{Error:¶ms.Error{"not found", "service \"bar\" not found"}}}} | 03:20 |
| davecheney | ... expected params.ErrorResults = params.ErrorResults{Results:[]params.ErrorResult{params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:¶ms.Error{"not found", "machine 42 not found"}}, params.ErrorResult{Error:¶ms.Error{"unauthorized access", "permission denied"}}, params.ErrorResult{Error:¶ms.Error{"unauthorized access", "permission denied"}}}} | 03:20 |
| davecheney | welcome to my day | 03:20 |
| davecheney | looking at long likes of quasi json and spotting the n differences | 03:20 |
| thumper | waigani: back | 03:46 |
| waigani | thumper: https://github.com/juju/juju/pull/553 | 03:46 |
| thumper | waigani: looking now, tests work ok? | 03:47 |
| wwitzel3 | waigani: I added the comments for the public methods on https://github.com/juju/juju/pull/511 | 03:48 |
| waigani | thumper: all tests passed except state/envuser_test.go which I fixed and tested individually | 03:49 |
| waigani | thumper: I'll do one more make test to be sure | 03:49 |
| waigani | wwitzel3: great thanks :) | 03:50 |
| waigani | wwitzel3, davecheney is my review mentor so you'll have to poke him for an LGTM | 03:52 |
| wwitzel3 | thanks :) | 03:52 |
| davecheney | two secs | 03:54 |
| thumper | waigani: you are missing the new files... | 03:55 |
| thumper | waigani: state/envuser.go + test are not there | 03:55 |
| thumper | wwitzel3: o/ | 03:55 |
| waigani | thumper: fuk | 03:55 |
| wwitzel3 | thumper: heya | 03:56 |
| waigani | thumper: state/envuser.go is already in master? | 04:00 |
| thumper | waigani: is it? | 04:00 |
| * thumper looks | 04:00 | |
| waigani | thumper: is for me? | 04:00 |
| thumper | waigani: umm... no. don't think so | 04:01 |
| waigani | thumper: okay my local master is stuffed up | 04:01 |
| waigani | thumper: let me clean this up and try again | 04:02 |
| thumper | ok | 04:02 |
| menn0 | thumper: that change you helped me with: https://github.com/juju/juju/pull/554/ | 04:20 |
| menn0 | thumper: do you have time to review? | 04:20 |
| thumper | sure | 04:20 |
| menn0 | thumper: i've already made sure this solves the problem of downgrades in local/manual envs | 04:21 |
| menn0 | so once this is in | 04:21 |
| menn0 | the other change can be proposed straight away | 04:21 |
| waigani | thumper: cleaned up, I'll address your initial comments before pushing again. | 04:23 |
| waigani | thumper: user take does not have a Name() method? | 04:23 |
| thumper | yes it does | 04:24 |
| thumper | are your deps up to date? | 04:24 |
| waigani | thumper: I just ran the godeps tool | 04:24 |
| thumper | but your branch probably isn't up to date with trunk | 04:24 |
| thumper | menn0: only one real comment | 04:26 |
| menn0 | thumper: I don't think what you've suggested is necessary | 04:29 |
| thumper | why? | 04:29 |
| menn0 | the test intentionally has no tools in env storage | 04:29 |
| menn0 | so if a download was attempted it would fail | 04:29 |
| menn0 | this work was done test first, and it failed because of this | 04:30 |
| waigani | thumper: dad duty just started, I'll get it back up later tonight. | 04:31 |
| thumper | waigani: ack | 04:31 |
| menn0 | thumper: does that make sense? | 04:31 |
| thumper | menn0: in which case, please add a comment :-) as it wasn't obvious to me | 04:31 |
| menn0 | thumper: will do | 04:31 |
| menn0 | thumper: added comment and rearranged the test a little to make things clearer | 04:37 |
| menn0 | how's this? https://github.com/juju/juju/pull/554/files | 04:37 |
| thumper | menn0: cheers | 04:37 |
| davecheney | wwitzel3: you've got a LGTM on that change | 04:38 |
| davecheney | there is nothing to be gained from another round of reviews | 04:38 |
| davecheney | do what you think is reasonable then submit that sukka before we get locked down again | 04:38 |
| thumper | menn0: a slightly premature merge :) | 04:40 |
| menn0 | thumper: sorry. I wasn't sure if you were going to say anything else | 04:40 |
| thumper | menn0: that's fine :) | 04:40 |
| menn0 | thumper: is there a problem | 04:40 |
| menn0 | ? | 04:40 |
| thumper | no | 04:40 |
| menn0 | just saw your LGTM :) | 04:40 |
| wwitzel3 | davecheney: thanks | 04:41 |
| menn0 | potentially dumb question: what does this do? go test -p 2 ./... | 04:43 |
| menn0 | and why does the build do it? | 04:43 |
| menn0 | wallyworld_'s last merge got aborted because a test took too long and then the tests were run again as above and passed | 04:44 |
| wallyworld_ | menn0: it runs the tests with reduced parallelism | 04:44 |
| wallyworld_ | because our tests sucks | 04:45 |
| menn0 | oh right | 04:45 |
| wallyworld_ | and often fail spuriously | 04:45 |
| menn0 | I didn't realise we were running tests in parallel at all! | 04:45 |
| wallyworld_ | but -p 2 works more often than not but is slower | 04:45 |
| wallyworld_ | menn0: that's what Go does out of the box | 04:45 |
| menn0 | right | 04:45 |
| menn0 | well I have learned something today | 04:45 |
| wallyworld_ | \o/ | 04:45 |
| menn0 | time to put my feet up | 04:46 |
| menn0 | :) | 04:46 |
| wallyworld_ | enjoy | 04:46 |
| davecheney | menn0: you and andrew gerrand | 05:01 |
| davecheney | he didn't know -p $NUMCPU was the default either | 05:01 |
| menn0 | davecheney: well I don't feel so bad then :) | 05:01 |
| davecheney | possibly he doesn't spend as long as you and I do waiting for tests to ru | 05:02 |
| * thumper goes to make dinner | 05:02 | |
| wwitzel3 | I'm off, be back in the morning o/ | 05:09 |
| === uru-afk is now known as urulama | ||
| davecheney | wallyworld_: ping | 06:26 |
| wallyworld_ | davecheney: hey | 06:55 |
| davecheney | wallyworld_: i have a question about the keymanager | 06:57 |
| davecheney | when we pass user: juju-system-key | 06:57 |
| davecheney | to the keymanager | 06:57 |
| davecheney | is that a tag ? | 06:57 |
| wallyworld_ | hmmm | 06:57 |
| wallyworld_ | i didn't add that bit, but let me look at the code | 06:57 |
| davecheney | i've had to put a horrible hack in to make juju accept that as a tag | 06:57 |
| davecheney | ok, ifyou didnd't writ it | 06:57 |
| davecheney | that is ok | 06:57 |
| davecheney | you have TODOs all over it | 06:58 |
| wallyworld_ | i don't think i did - i wrote the key manager itself | 06:58 |
| wallyworld_ | but | 06:58 |
| davecheney | i have exactly one test to fix | 06:58 |
| davecheney | then this bloody branch is done | 06:58 |
| wallyworld_ | i think someone came along and added support for a system-key | 06:58 |
| davecheney | well, they cocked it up | 06:58 |
| wallyworld_ | let me look briefly | 06:58 |
| wallyworld_ | i could have been me, but i can't recall | 06:58 |
| wallyworld_ | davecheney: looks like it was me. a key can be for a user eg fred, or is the juju system key | 07:02 |
| davecheney | right, so they aren't tags | 07:02 |
| wallyworld_ | i guess that was done back before tags | 07:02 |
| davecheney | yeah | 07:02 |
| davecheney | which makes it a bit of a hack to use common.AuthFunc | 07:03 |
| davecheney | anyway | 07:03 |
| davecheney | i've done a gross hack | 07:03 |
| davecheney | it'll hold | 07:03 |
| wallyworld_ | yuk, sorry | 07:03 |
| davecheney | meh | 07:03 |
| davecheney | we'll live | 07:03 |
| davecheney | it can't be fixed now because of BACKWARDS COMPATABILITY !!1! | 07:03 |
| wallyworld_ | yep :-( | 07:04 |
| davecheney | wallyworld_: https://github.com/juju/juju/pull/556 | 07:07 |
| davecheney | DONE | 07:07 |
| davecheney | it's only taken a month | 07:07 |
| davecheney | i'm off to the pub | 07:07 |
| wallyworld_ | looking | 07:07 |
| wallyworld_ | have a drink for me :-) | 07:07 |
| voidspace | perrito666: let me know when you arrive :-) | 07:09 |
| jam | wallyworld_: I'm pretty sure you'll be having a drink for yourself, too :) | 08:14 |
| TheMue | morning | 08:16 |
| wallyworld_ | jam: yes i will :-) | 08:18 |
| jam | morning TheMue | 08:18 |
| TheMue | dimitern: ping | 08:22 |
| dimitern | TheMue, hey | 08:22 |
| TheMue | dimitern: I changed my approach a bit, but I still hesitate to use the Entity approach. | 08:23 |
| TheMue | dimitern: see https://github.com/TheMue/juju/compare/capability-detection-for-networker | 08:23 |
| dimitern | TheMue, looking | 08:23 |
| TheMue | dimitern: Entity is really generic, for all entities with a tag. But IsManual is a flag for machines. | 08:24 |
| TheMue | dimitern: I now read it only once when creating a local Machine instance, so it is cached and will never bee refreshed, because it won’t change. | 08:25 |
| jam | TheMue: AIUI Jobs is also specific to Machines, but the "thing" is Entity which is more generic | 08:25 |
| dimitern | TheMue, no, please let's not make 2 calls for every life request | 08:25 |
| TheMue | jam: Entity doesn’t know about jobs | 08:26 |
| dimitern | TheMue, yes jam's right - there's already a precedent with Jobs, adding another flag is not a big deal I think | 08:26 |
| jam | dimitern: themue seems to think I've got the wrong object, I could be wrong there. | 08:27 |
| jam | But I thought I remembered seeing other stuff that was machine specific on the generic class | 08:27 |
| TheMue | Aaargh, I see, we’re talking about AgentGetEntitiesResult | 08:28 |
| dimitern | yes! :) | 08:29 |
| TheMue | But I’m still not happy with overloading that struct more and more | 08:29 |
| dimitern | TheMue, that's what versioning is for, no? | 08:29 |
| TheMue | dimitern: I don’t talk about differences, I talk about common and specific fields, separation of concerns | 08:30 |
| dimitern | TheMue, the agents need this data to do their jobs (efficiently), and it's also an internal api | 08:30 |
| TheMue | dimitern: So I would be more happy to let the machine fetch a doc especially for a machine | 08:30 |
| TheMue | dimitern: still not clean and optimal for maintainability, you see how easy one can move into the wrong direction | 08:31 |
| dimitern | TheMue, I don't think this is us moving in the wrong direction | 08:31 |
| TheMue | dimitern: IMHO the Machiner would be responsible for Machine operations and the Machine in the should know what it has to know about machines | 08:32 |
| dimitern | TheMue, fwiw, we probably need to have 2 separate unit and machine agent facades, rather than pretending we can treat both and their entities the same | 08:32 |
| dimitern | TheMue, and if you look at AgentGetEntitiesResults, only Life is common between machines and units, ContainerType and Jobs only apply to machines | 08:34 |
| TheMue | dimitern: yeah, maybe. so to come to a quick solution I will do the AgentGetEntitiesResult change. and I’ll file an issue for refactoring with low priority | 08:35 |
| dimitern | TheMue, that sounds good to me! | 08:37 |
| voidspace | So "juju restore" bootstraps again (which it needs to) | 08:52 |
| voidspace | but if it fails it leaves the instance created - but with no running agent. | 08:52 |
| voidspace | So "juju status" (etc) fail and you have to manually destroy the instance (or accidentally leave it running because you don't realise)... | 08:53 |
| voidspace | jam: your hacked-up-backup script (for local provider) | 08:53 |
| voidspace | jam: it uses $JUJU_ENV | 08:53 |
| voidspace | this isn't set when I'm running the script directly, so I have to set it myself I guess | 08:53 |
| voidspace | or would you expect it to be set? | 08:53 |
| jam | voidspace: I believe it is set if you run "juju restore" but I just use "JUJU_ENV=local ./juju-restore" | 08:54 |
| voidspace | jam: right, cool | 08:54 |
| jam | voidspace: if you just run "juju-backup" it fails too, right? | 08:54 |
| jam | My understanding is that the plugin infrastructure sets that variable | 08:54 |
| voidspace | jam: it fails for publiic-key | 08:54 |
| voidspace | jam: right | 08:54 |
| voidspace | jam: that's what I *thought*, but I as just checking my system wasn't screwed in new and exciting ways | 08:54 |
| jam | voidspace: did you do any more hacking on it, or shall I see if I can get it through to completion? | 09:01 |
| voidspace | jam: I was trying (and failing) to get amazon working | 09:01 |
| voidspace | jam: I've now switched to the local script | 09:01 |
| voidspace | backup seems to work fine :-) | 09:01 |
| dimitern | jam, got a minute? | 09:04 |
| jam | voidspace: well local backup "works" in that it generates a tarball, but all the paths match exactly the local paths, which means they *don't* match the "official" paths. | 09:04 |
| dimitern | jam, I'd like to pass an idea by you | 09:04 |
| jam | well, what *I* would consider official, at least (/var/lib vs /home/$USER/.juju/etc) | 09:04 |
| voidspace | jam: right | 09:04 |
| voidspace | yep | 09:04 |
| jam | dimitern: G+ or just in IRC? | 09:05 |
| dimitern | jam, just IRC should be fine | 09:05 |
| jam | voidspace: I think the thing to do there would actually be to rewrite them, but I don't expect backup of Amazon to be able to restore into local | 09:05 |
| voidspace | sure! | 09:05 |
| voidspace | just local to local will be a good start | 09:05 |
| voidspace | local to amazon might be useful :-) | 09:06 |
| dimitern | jam, so after experimenting with the net package's Interfaces() and InterfaceAddrs() on precise, trusty and utopic daily image, I realized we can get all the information we need about the network interfaces *without* ever bothering to read /etc/network/interfaces or /interfaces.d/*.cfg | 09:06 |
| voidspace | create your system locally, then "restore" to your production systems | 09:06 |
| jam | dimitern: what is it that we wanted but don't have access to? | 09:06 |
| dimitern | jam, like what? | 09:07 |
| jam | "we can't get all the information we need" such as? | 09:07 |
| jam | looking here: http://golang.org/pkg/net/#Interface | 09:07 |
| jam | it looks like we don't have anything about bridging, etc. | 09:07 |
| dimitern | jam, we can discover all interfaces, bridges included, their flags (up, down, broadcast, loopback, multicast), and their index and names, also any addresses assigned | 09:08 |
| dimitern | jam, I'm proposing to just rewrite the interfaces config as needed for ifup/down to work at boot, and as the networker starts it will read all configs and check if there are any changes between what we have in state and what's configured on the machine, and rewrite the configs as needed | 09:09 |
| jam | dimitern: ok, I read what you said wrong. I read it as "we cannot get all the information", if we *can* that's great, though I have my doubts. I'm also wondering where "net" gets its information from, is it just the live state, or is it the state that will be started on restart | 09:10 |
| dimitern | jam, the net package gets the info using syscalls and some obscure netlink packets | 09:11 |
| jam | dimitern: k, so it would be the "live" state, vs the saved state | 09:11 |
| dimitern | jam, it is more or less equivalent to what we can see using the "ip" command (i.e. real-time) | 09:11 |
| jam | dimitern: right, it is a kernel sort of interface | 09:11 |
| jam | "ifup" and /etc/network/interfaces are a debian/ubuntu-ism | 09:11 |
| dimitern | jam, yes, and it works just as well in precise as in utopic | 09:11 |
| jam | IIRC, Fedora/Redhat use a different saved state | 09:11 |
| dimitern | jam, well, there is support only for linux to discover the interfaces like that, but I've yet to try centos and fedora | 09:12 |
| dimitern | jam, that's true, we'll have to render different stored config for other distros, but let's worry about this as we get there :) | 09:13 |
| dimitern | jam, another up-side of using the net package is we don't need to rely on /etc/network/interfaces being there to decide whether to start the networker or not (more so now, as we have the "safe mode" to disable it selectively) | 09:15 |
| jam | dimitern: sorry about the delay, emergency take-the-dog-out action. | 09:38 |
| jam | dimitern: so I like the idea a bit (no safe mode would be very nice) | 09:38 |
| jam | *but* | 09:38 |
| jam | I wonder if we really don't want to save our configuration, which means writing to /etc/network/* | 09:38 |
| dimitern | jam, no, no, I'm not saying that :) | 09:39 |
| jam | dimitern: perhaps we could only rewrite things when we've actually decided we need to change things | 09:39 |
| dimitern | jam, we do write the network config in /etc, but we don't read it - we use the net package to discover what nics are there | 09:39 |
| jam | dimitern: but we shouldn't write it unless we think there is a change from what's written there, should we? | 09:40 |
| jam | in which case, we have to read what is there to write anythnig | 09:40 |
| dimitern | jam, we need to write the config, otherwise without a working juju machine agent + networker, the networking won't be setup correctly after system boot | 09:40 |
| dimitern | jam, yes, we get all NICs, read what's there and render and write "managed" config for each nic that we need to setup, but only if it's pre-existing config on disk is different from what the networker would generate | 09:41 |
| dimitern | jam, am I making more sense? :) | 09:42 |
| jam | dimitern: so I can understand that we may not need to parse them, I'm not 100% sure that our method of comparison as byte-strings is quite what we want (what about comments in the files, etc). | 09:43 |
| dimitern | jam, ok, so let's backtrack just a bit | 09:43 |
| dimitern | jam, the user trusts (or we hope they do) juju to do the right thing with the machines juju manages, right? the same should be true wrt the networker taking control over the machine's networking config and managing it, without expecting the user to interfere? | 09:44 |
| jam | dimitern: the whole point of "Safe" is because we don't trust us on Manual or Local | 09:45 |
| dimitern | jam, we make this obvious by using the comment header "# Managed by Juju, please do not change." in each config file the networker generates | 09:45 |
| dimitern | jam, right, so for "safe mode", don't write changes to the config or run any ifup/down commands, effectively just sitting there and logging what we discovered | 09:46 |
| dimitern | jam, I'm hoping we can get rid of safe mode for manual machines once we have a way for the user to explicitly tell us how to setup networking on them, or we're able to discover it somehow, smartly | 09:50 |
| jam | dimitern: TheMue: voidspace: standup? | 10:45 |
| voidspace | jam: omw | 10:45 |
| voidspace | perrito666: ping | 10:50 |
| jam | voidspace: are you still there? | 10:52 |
| jam | marcoceppi: poke about charm tools: https://bugs.launchpad.net/charm-tools/+bug/1359170 | 11:34 |
| mup | Bug #1359170: juju charm create fails in an unhelpful fashion with juju-1.21 in PATH <Juju Charm Tools:New> <https://launchpad.net/bugs/1359170> | 11:34 |
| jam | Also, when I look at: https://juju.ubuntu.com/docs/authors-charm-writing.html | 11:34 |
| jam | It has the lines: | 11:34 |
| jam | juju charm create vanilla | 11:34 |
| jam | but in my browser, those are wrapped onto 2 lines | 11:34 |
| jam | so it looks like 2 commands | 11:34 |
| jam | Ctrl Refresh fixed it for me.... very weird | 11:35 |
| perrito666 | morning voidspace | 11:41 |
| voidspace | perrito666: morning | 11:41 |
| voidspace | perrito666: solved my horrendous backup / restore problems | 11:41 |
| voidspace | perrito666: well, more or less | 11:42 |
| perrito666 | voidspace: heh, all backup and restore problems are horrendous | 11:42 |
| voidspace | perrito666: backup wasn't working *at all* | 11:42 |
| perrito666 | voidspace: odd, since when? | 11:43 |
| voidspace | perrito666: but jam finally sussed it - the juju 1.18 juju-backup is in the path and the dev one isn't | 11:43 |
| voidspace | perrito666: only for me on trunk | 11:43 |
| voidspace | perrito666: so I have to run the backup script directly to try it | 11:43 |
| perrito666 | sweet, is there a PR already? | 11:46 |
| voidspace | perrito666: not a PR yet | 11:46 |
| voidspace | perrito666: I'll show you what I have (complete now I think) | 11:47 |
| jam | perrito666: the issue is that the 1.18 "juju-backup" shell script didn't know about /usr/lib/juju/bin/mongodump which you fixed for 1.20, but 1.18 is the one in /usr/bin/juju | 11:47 |
| voidspace | perrito666: although I need to test it, hence the problem with backup not working | 11:47 |
| voidspace | perrito666: https://github.com/voidspace/juju/compare/backup-ssh | 11:47 |
| jam | and just running "juju backup" searches $PATH for "juju--backup" so even though juju itself is 1.21, "juju backup" was running the 1.18 version. | 11:48 |
| voidspace | jam: so backup and restore from trunk worked fine for me - which is a first | 11:48 |
| voidspace | now to try it with my code... | 11:48 |
| perrito666 | ah very true, my usual test uses absolute path | 11:48 |
| jam | perrito666: you mean /home/user/path/to/juju/cmd/plugins/juju-backup/juju-backup ? | 11:50 |
| perrito666 | jam: yup, since I use the automated test from qa I added my paths to make sure it alwas runs the same | 11:52 |
| jam | perrito666: and then presumably you just set JUJU_ENV to test the right environment? | 11:52 |
| jam | TheMue: didn't you land a patch to allow "@" syntax for "juju set" ? | 11:53 |
| perrito666 | jam: the CI test requires it as a param actually (test_recovery.py) | 11:54 |
| jam | perrito666: I don't see a test_recovery.py in lp:juju-ci-tools | 11:56 |
| jam | test_assess_recovery.py ? | 11:56 |
| perrito666 | jam: it was test_recovery.py until friday | 11:57 |
| * perrito666 pulls | 11:57 | |
| jam | perrito666: perhaps it is a different source code, because test_assess_recovery.py imports "test_recovery" at the top. | 11:57 |
| perrito666 | RM test_recovery.py => assess_recovery.py | 11:58 |
| jam | perrito666: looks like test_assess_recovery.py didn't get fixe | 11:58 |
| jam | fixed | 11:58 |
| jam | perrito666: and it looks like CI does, indeed, run "juju-backup" and not "juju backup" which might be something we should think carefully about, since *users* are going to be doing the latter. | 11:59 |
| jam | but we are testing the former | 11:59 |
| perrito666 | I must have local changes to ensure that, ah I see, those tests ship their own version of backup | 11:59 |
| * perrito666 wonders why | 12:00 | |
| voidspace | interesting | 12:00 |
| voidspace | yeah, seems odd | 12:00 |
| perrito666 | jam: good point Ill nag sinzui about it when he shows | 12:00 |
| voidspace | anyway, about to test my restore | 12:00 |
| voidspace | just killed the state server | 12:00 |
| voidspace | jam: looks like restore does fetch the tools from the correct storage location when used in conjunction with upload-tools (just btw) | 12:04 |
| jam | voidspace: good to hear. I expected as much, since we are using the control-bucket as defined in the .jenv | 12:04 |
| jam | I almost wonder if backup should be saving the .jenv as well. | 12:05 |
| jam | since you really need both (I believe) | 12:05 |
| voidspace | yep | 12:05 |
| voidspace | jam: so my restore failed | 12:08 |
| jam | voidspace: did stock juju core succeed? | 12:08 |
| voidspace | jam: it looks like it opened the apiState fine, but then the Client failed to get an address for "machine-1" (my mysql unit - so the call to client.PublicAddress("machine-1") failed | 12:08 |
| voidspace | jam: yes | 12:08 |
| voidspace | jam: heh, although to be fair I didnt try with a unit with trunk as I wanted a quick test | 12:09 |
| voidspace | but this seems like a plausible failure | 12:09 |
| voidspace | I'm going to make coffee and then dig in | 12:09 |
| voidspace | no public address found: unknown unit or machine "machine-1 | 12:09 |
| jam | voidspace: the nice thing about backup and restore is that once you have the backup, you don't have to backup again, just try restore | 12:10 |
| voidspace | yep :-) | 12:10 |
| jam | voidspace: that sounds like a "tag" vs an "id" issue | 12:10 |
| jam | as in, it might be expecting "1" for "machine-1" | 12:10 |
| voidspace | ah right, just "1" maybe | 12:10 |
| voidspace | will try that | 12:10 |
| voidspace | even before coffee :-) | 12:10 |
| voidspace | although if restore fails you have to manually kill the ec2 instance | 12:12 |
| voidspace | as a failed restore leaves the instance it created running | 12:13 |
| TheMue | jam: sorry, just seen your question. have been to lunch. | 12:15 |
| voidspace | ok, restore underway again | 12:16 |
| voidspace | coffee time | 12:16 |
| TheMue | jam: have to see what it has been exactly, but yes, IMHO it has been the @ to lead from files | 12:16 |
| TheMue | jam: https://bugs.launchpad.net/juju-core/+bug/1216967 | 12:18 |
| mup | Bug #1216967: Missing @ syntax for reading config setting from file content <compat> <docs> <hours> <improvement> <juju-core:Fix Committed by themue> <https://launchpad.net/bugs/1216967> | 12:18 |
| jam | TheMue: k, I thought it would show up in "juju help set" but I didn't see it there | 12:18 |
| TheMue | jam: interesting, afair it should | 12:19 |
| TheMue | jam: the set doc has a hint. three paragraphs, the second one. which version do you use? | 12:21 |
| jam | TheMue: http://paste.ubuntu.com/8097539/ | 12:23 |
| jam | is all I see for "juju help set | 12:23 |
| TheMue | jam: see line 28 in juju/cmd/set.go, it’s more. but it’s not yet released | 12:25 |
| TheMue | jam: so if you don’t use your own build you won’t see it. | 12:25 |
| jam | TheMue: $ ~ | 12:26 |
| jam | ~/dev/go/bin/juju set -h | 12:26 |
| jam | I'm using trunk | 12:26 |
| jam | ~/dev/go/bin/juju --version | 12:26 |
| jam | 1.21-alpha1-trusty-amd64 | 12:26 |
| jam | TheMue: you have "setDoc" set to the correct thing | 12:26 |
| jam | but line 48 | 12:26 |
| jam | has "Doc: "set one or more ..." | 12:27 |
| jam | not Doc: setDoc | 12:27 |
| jam | TheMue: care to submit a bug and a patch for it? | 12:27 |
| TheMue | jam: aaargh, I found setDoc before and expanded it. never seen that it isn’t used. | 12:27 |
| TheMue | jam: yep, will quickly do it. | 12:27 |
| voidspace | jam: perrito666: looks to me like restore worked fine! | 12:46 |
| perrito666 | voidspace: sweet | 12:46 |
| voidspace | jam: perrito666: will double check the code and create PR | 12:46 |
| voidspace | and then *re-PR* the database shutoff | 12:47 |
| jam | voidspace: greate. why didn't it work the first time? (did you fix something?) was it just the "machine-1" vs "1" issue? | 12:47 |
| voidspace | jam: yep | 12:47 |
| voidspace | jam: switching to id instead of tag fixed it | 12:48 |
| jam | voidspace: I wish our code was more consistent here, but that is what API versioning could get us to :) | 12:48 |
| jam | voidspace: can you file a bug tagged "API" about the inconsistency? | 12:48 |
| jam | and tech-debt | 12:48 |
| voidspace | jam: yep, and I'm adding a note to the PublicAddress docstring that "target" is an id not a tag. | 12:49 |
| perrito666 | aghh the convoluted settings of networkmanager/dnsmasq in 14.04 drive me crazy | 12:51 |
| jam | perrito666: it looks like we broke plugins in 1.21 as well, which would have been caught if we were doing "juju restore" instead of "juju-restore", not that we should have used that to actually catch the bug | 12:52 |
| perrito666 | jam: thats huge, dont we have a test to make sure plugins actually works? | 12:53 |
| jam | perrito666: apparently not https://bugs.launchpad.net/juju-core/+bug/1359170 | 12:53 |
| mup | Bug #1359170: juju charm create fails in an unhelpful fashion with juju-1.21 in PATH <Juju Charm Tools:New> <juju-core:Triaged> <https://launchpad.net/bugs/1359170> | 12:53 |
| jam | I tried to run the charm-tools plugin | 12:53 |
| jam | and apparently we stopped passing arguments to plugins in 1.21 | 12:54 |
| jam | I'm testing 1.20 now | 12:54 |
| jam | it seems broken there, too | 12:54 |
| jam | bugger | 12:54 |
| jam | curtis won't be happy | 12:54 |
| jam | wallyworld_: can I point you to bug #1359170 when you're around? | 12:56 |
| mup | Bug #1359170: arguments no longer passed to plugins <Juju Charm Tools:Invalid> <juju-core:Triaged> <juju-core 1.20:Triaged> <https://launchpad.net/bugs/1359170> | 12:56 |
| jam | I'll try to pick it up since I discovered it | 12:56 |
| jam | CI isn't failing, so I'm not sure if it should be "CI regression" and block trunk, but it is a *serious* f-up on our part. | 12:58 |
| jam | I can't quite see why it is failing yet, though. | 12:59 |
| jam | sinzui: you'll certainly be happy to hear that I just opened a critical bug against 1.20... | 13:02 |
| sinzui | :) | 13:02 |
| jam | sinzui: bug #1359170 | 13:03 |
| mup | Bug #1359170: arguments no longer passed to plugins <regression> <Juju Charm Tools:Invalid> <juju-core:Triaged by jameinel> <juju-core 1.20:Triaged by jameinel> <https://launchpad.net/bugs/1359170> | 13:03 |
| sinzui | jam, That is nicer that something breaking the entire packaging and publication step. I think you know what is wrong in you bug. at this moment CI is dead and I don't know why | 13:04 |
| TheMue | Hmm, it’s more intelligent to push branches when planing to create a PR. | 13:04 |
| sinzui | abentley, mgz: Looks like by stress from yesterday was just waiting for me to return to the job this morning. My top priority is to backport some ubuntu packaging rules into our source package branch. We want 1.20.5 accepted into utopic by feature freeze this Friday | 13:07 |
| abentley | sinzui: Hoo boy. Have fun. | 13:08 |
| sinzui | Looks like one of the packaging changes broke CI. I will revert, but I think we need to rerun 1.20 each time I want to verify my changes to the source package branch | 13:09 |
| TheMue | jam: https://github.com/juju/juju/pull/559, extreme trivial :D | 13:09 |
| abentley | sinzui: Is "/home/ubuntu/juju-build/*.deb" right? Should it be in the jenkins home dir? | 13:10 |
| jam | TheMue: I'd like to have a Launchpad bug to reference for this fix, also, you can add a test case in test_main.go which asserts the output of "juju set -h" matches setDoc. | 13:10 |
| jam | TheMue: otherwise LGTM | 13:11 |
| sinzui | abentley, no, because the script was written to work on a remote instance. | 13:11 |
| TheMue | jam: OK, the bug already exists, will put it in the description. And will add a test. Do we test the doc output in other commands too? Otherwise we should add tests there too. | 13:12 |
| sinzui | abentley, I added a hack to the script to run on local host to use the jenkin's workspace...but we haven't dismantled publish-revision to build on separate machines. | 13:12 |
| jam | TheMue: I've written tests in "main_test.go" before in order to assert that things were actually registered | 13:12 |
| jam | TheMue: for example "juju sync-tools --help" is in there. | 13:13 |
| jam | TheMue: *I* did it, because it was the most obvious place to check the lines in main.go where we have "r.Register(&Command)" | 13:13 |
| jam | TheMue: (test that the command exists because we get proper help text for it, rather than trying to say run the command and have unwanted side effects) | 13:14 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1359170 | ||
| TheMue | jam: fine, so should look if this is done for all commands (but in an extra change) | 13:15 |
| jam | ah ffs, things work *if* you pass an environment name in "-e".... | 13:16 |
| sinzui | jam: I would like your advise. backup restore doesn't work unless mongodb-clients is not installed on the client machine. I think this is a " Depends" relationship in packaging. It was suggested to me that I use "recommends" because it is not needed by everyone. | 13:18 |
| jam | sinzui: if having something installed/not installed on the *client* changes whether backup + restore works then we just have a bug. | 13:20 |
| jam | we should not depend on local stuff in order for us to run stuff on the server. | 13:20 |
| jam | sinzui: it sounds like we are doing some sort of "does X exist" check on the client, when it needs to be run on the server. | 13:21 |
| perrito666 | sinzui: mongodb-clients isrequired on the server not the client | 13:21 |
| sinzui | perrito666, oh! | 13:22 |
| perrito666 | sinzui: and it is only required for restore | 13:22 |
| perrito666 | and restore installs it if its not there | 13:22 |
| sinzui | perrito666, fab. Thank you very much perrito666 , so I am going to mark the bug as fixed in 1.20.x by you | 13:23 |
| perrito666 | sinzui: indeed, the bug you just marked as fixed has the wrong diagnose, but it has been fixed in 1.20 | 13:27 |
| sinzui | perrito666, Yeah. I elected to show it fixed in the latest to encourage people to seek it out | 13:28 |
| perrito666 | sinzui: jam talked about it earlier, its because backup in 1.18 does not do a proper lookup for the tools which are in a juju folder | 13:28 |
| jam | dimitern: you're OCR today, so https://github.com/juju/juju/pull/560 if you want to give it a look. It needs a test that we operate properly under the circumstances I described in the bug. | 13:34 |
| jam | dimitern: I'd also like to run it past thumper, since I'm pretty sure he wrote the environmentCommandWrapper stuff. | 13:35 |
| * sinzui restarts CI from the publish-revision step for master | 13:35 | |
| dimitern | jam, sure, will have a look in a bit | 13:40 |
| === gsamfira1 is now known as gsamfira | ||
| jam | perrito666: so it turns out we wouldn't have discovered my bug by running "juju restore", because it only breaks Args when you don't have an Environment, and "juju-restore" must have an environment set anyway. | 13:48 |
| perrito666 | jam: you do make an interesting case for a test anyway | 13:48 |
| jam | perrito666: so we might have a test, but it either has a default environment, or what it is testing requires one. | 13:48 |
| jam | perrito666: I may be unique in how I do my environment management, and if I'm unique enough, maybe we should just deprecate it and not actually support it | 13:49 |
| jam | either that, or we need to be doing more testing that my way is actually supported. | 13:49 |
| jam | bash autocompletion also was (is?) buggy for me because it assumed you'd have an environment set | 13:49 |
| perrito666 | jam: well, if we support it, we need to test it | 13:49 |
| alexisb | natefinch, wwitzel3: TOSCA call? | 14:02 |
| natefinch | alexisb: coming | 14:02 |
| wwitzel3 | alexisb: I thought I was already in it .. | 14:02 |
| wwitzel3 | alexisb: restarting the hangout :) | 14:02 |
| dimitern | jam, reviewed; just one comment - sorry it took so long :/ | 14:05 |
| ericsnow | dimitern: you have a minute to take a look at a PR? https://github.com/juju/juju/pull/530 | 14:30 |
| alexisb | TheMue, on my way, but will be a few minutes late | 15:01 |
| TheMue | alexisb: ok | 15:01 |
| ericsnow | perrito666, natefinch, wwitzel3: standup? | 15:06 |
| perrito666 | ericsnow: going, just fighting with my hangut | 15:07 |
| === urulama is now known as uru-afk | ||
| natefinch | email I just got: "This is to notify you that the sum of 800,000 Britain Currency has been Donated to you by Colin Chris Weir Donation Scheme. for claims provide the below details" | 15:39 |
| gsamfira | ahh, they toned it down a bit. Used to be 10X that :P | 16:09 |
| natefinch | I just though "britain currency" was cute | 16:19 |
| natefinch | Also, funny they call it a "donation scheme" | 16:19 |
| natefinch | fwereade: at some point I'd like to pick your brain about charm feature flags. I was hoping to work on it this week, but I'm less certain now, after talking to rick_h__ about it. | 16:25 |
| natefinch | sinzui: did you make progress on getting godeps in the tarball? No big deal if it's not in yet, just checking. | 16:25 |
| fwereade | natefinch, dammit, I wanted to talk to you today, but the babysitter's just arrived and I'm trying to finish an email | 16:25 |
| fwereade | natefinch, what time is it for you now? | 16:25 |
| natefinch | fwereade: 12:30 | 16:25 |
| fwereade | natefinch, cool, I hope I can get back to you in the next 5.5h | 16:26 |
| natefinch | fwereade: cool, ping me when you have time. I appreciate you making time for it in your evening. | 16:26 |
| sinzui | natefinch, I failed. I cannot make it run run in unittests. But this morning I realised I want to run that as a precheck before the tarball is created. I hope to have that sorted out this week. Sorry, it wasn't an easy change, and I need to focus on putting 1.20.5 into utopicbefore Friday | 16:28 |
| alexisb | jam, you doing the cloudsigma call today | 16:31 |
| natefinch | sinzui: no problem. It's not pressing. It's a nice to have. What I can do is make the test skip itself for now if godeps doesn't exist, that way it can still help developers (who should have godeps). | 16:39 |
| sinzui | natefinch, thank you that works for me because godeps does exist when we make the tarball. | 16:46 |
| rogpeppe1 | natefinch: did you consider making that godeps test part of CI rather than an actual test in the suite? | 16:49 |
| natefinch | rogpeppe1: it's at least as important for developers as CI..... since innumerable times I've seen people ask why something was failing, only to figure out later that their dependencies were out of date | 16:49 |
| natefinch | (which has happened to me, as well) | 16:50 |
| natefinch | sinzui: I would like to make it mandatory at some point. If you're developing on juju, you *need* godeps... I don't want someone to start hacking on juju and wonder why it's broken, just because they haven't run godeps. | 16:51 |
| sinzui | ericsnow, I am indeed very busy today I will reply before I EOD. Hp is my preferred could this week, and the the juju-core-slave machine might be ideal for setting up review board there. I need to think about your questions in more details though | 16:52 |
| ericsnow | sinzui: no worries :) | 16:52 |
| rogpeppe1 | natefinch: ok. BTW, in your test, why not just use diff? | 16:54 |
| natefinch | rogpeppe1: what diff? | 16:57 |
| rogpeppe1 | natefinch: diff(1) | 16:57 |
| natefinch | rogpeppe1: windows? | 16:57 |
| rogpeppe1 | natefinch: no diff on windows? | 16:58 |
| natefinch | rogpeppe1: lol, no :) | 16:58 |
| gsamfira | git diff works :P | 16:58 |
| gsamfira | or were you referring to the manual? :P | 16:58 |
| gsamfira | in which case...no :D | 16:59 |
| rogpeppe1 | gsamfira: i was referring to a command-line diff tool. i guess git has access to one, but perhaps it's not available for general use | 16:59 |
| gsamfira | it is available | 17:00 |
| rogpeppe1 | natefinch: ^ | 17:00 |
| gsamfira | if you have Git installed its in C:\Program Files (x86)\Git\bin\diff.exe | 17:01 |
| gsamfira | along with less, grep, etc | 17:01 |
| gsamfira | a small slice of home when having to work on windows | 17:01 |
| rogpeppe1 | gsamfira: hmm, but i guess that won't be in the path, so you'd need some custom logic to work out how to find it (perhaps it's not in C:) | 17:01 |
| gsamfira | setx PATH "%PATH%;C:\Program Files (x86)\Git\bin" | 17:02 |
| gsamfira | and there ya go | 17:02 |
| natefinch | the last thing I want is yet another external dependency | 17:02 |
| gsamfira | if you set up your own system | 17:02 |
| gsamfira | you know where it is :) | 17:02 |
| rogpeppe1 | natefinch: out of interest, what proportion of our tests pass under windows? | 17:02 |
| gsamfira | well, yes...an external dependency sucks :) | 17:02 |
| rogpeppe1 | natefinch: you're probably right. but you could reduce your "monstrosity" by factoring out a "diff" function (diff(s, t string) []string, or something | 17:03 |
| natefinch | rogpeppe1: a bunch, and that's next on our list of things to do - get tests passing on windows (except those we deem we can skip on windows because we just can't support them, like containers) | 17:03 |
| natefinch | rogpeppe1: yep | 17:03 |
| gsamfira | most tests on Windows fail because of the same reasons: unsanitized paths, short paths vs long paths, directory names containing illegal characters on windows (like ":") | 17:05 |
| gsamfira | there will be a few pull requests this week to fix some of them | 17:07 |
| gsamfira | the new github.com/juju/juju/wrench package also breaks tests on windows | 17:07 |
| natefinch | wwitzel3: want to do our 1:1? | 17:19 |
| wwitzel3 | natefinch: yep | 17:19 |
| === uru-afk is now known as urulama | ||
| natefinch | heh, I had to fix dependencies.tsv on my PR that tests dependencies.tsv, because it was wrong. | 18:07 |
| wwitzel3 | :) | 18:09 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1359170, 1359333 | ||
| sinzui | natefinch, can you find an engineer to look into bug 1359333 | 18:58 |
| mup | Bug #1359333: win client thinks the cloud-init log is on the c drive <ci> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1359333> | 18:58 |
| natefinch | sinzui: crud. Yep. | 18:59 |
| natefinch | fwereade: forgot about the all team meeting tonight, I'm going to have to run to get some stuff done before then. Hopefully we can talk in the morning. | 20:06 |
| thumper | (╯°□°)╯︵ ┻━┻ | 21:51 |
| wwitzel3 | yep | 21:51 |
| thumper | menn0: still need that review? | 21:59 |
| thumper | can do it while all tests running | 21:59 |
| menn0 | thumper, yes pleased | 21:59 |
| menn0 | please even | 21:59 |
| menn0 | (urgh... I have had just over 2 hours sleep - sick kids suck) | 21:59 |
| thumper | menn0: trade you https://github.com/juju/juju/pull/563 | 22:02 |
| menn0 | thumper, done | 22:02 |
| thumper | menn0: that was quick, did you read it ? | 22:03 |
| * thumper pokes | 22:03 | |
| menn0 | thumper, no I meant "yes, that trade sounds fair" | 22:03 |
| thumper | oh | 22:03 |
| thumper | heh | 22:03 |
| menn0 | tiiiiiired | 22:03 |
| ericsnow | menn0: when you get a chance could you take a look at https://github.com/juju/juju/pull/530 | 22:04 |
| menn0 | the fun thing about me reviewing your change is that you then have to review my review :) | 22:04 |
| thumper | heh | 22:04 |
| alexisb | menn0, heh | 22:04 |
| thumper | hmm | 22:04 |
| menn0 | thumper, maybe we ask dave or someone else to do that for this situation | 22:05 |
| menn0 | ericsnow, will do | 22:05 |
| ericsnow | menn0: thanks! | 22:06 |
| menn0 | let me get this review done and another small PR up first | 22:06 |
| menn0 | thumper, regarding your PR... is it safe to assume that people won't/can't upgrade directly from pre 1.18 to post 1.20? | 22:08 |
| menn0 | thumper, they have to go through an intermediate 1.18 release first? | 22:08 |
| thumper | menn0: yes | 22:08 |
| menn0 | actually 1.18 and 1.20 | 22:08 |
| thumper | that has always been the approach | 22:09 |
| thumper | the idea with the upgrade steps is that now it is *possible* to jump | 22:09 |
| thumper | but we don't support it yet | 22:09 |
| menn0 | thumper, is that enforced? | 22:09 |
| menn0 | thumper, found the bit that enforces it: validate() in upgradejuju.go | 22:12 |
| thumper | not explicitly I think | 22:12 |
| menn0 | shame that it's enforced on the client side | 22:12 |
| thumper | oh, good :) | 22:12 |
| menn0 | thumper, with the new release number scheme it's not quite right either... | 22:13 |
| menn0 | thumper, the logic tries to find the next stable version by bumping minor by 1 if the current version is a dev release and by 2 otherwise | 22:15 |
| menn0 | thumper, that's no longer right is it? | 22:15 |
| thumper | no | 22:15 |
| thumper | please file a bug | 22:15 |
| menn0 | thumper, already doing it :) | 22:16 |
| menn0 | thumper, when does the new scheme start? 1.20? i.e. will the next stable release be 1.21? | 22:17 |
| thumper | menn0: yes | 22:17 |
| thumper | 1.21 will be a stable release | 22:17 |
| waigani | thumper: talking of reviews: https://github.com/juju/juju/pull/553 | 22:18 |
| * thumper nods | 22:18 | |
| menn0 | thumper, please check the milestones and priorities for bug 1359435 | 22:24 |
| mup | Bug #1359435: Next version selection for upgrades is no longer correct <juju-core:New> <juju-core 1.20:New> <https://launchpad.net/bugs/1359435> | 22:24 |
| menn0 | thumper, review done. | 22:28 |
| * thumper does a bootstrap | 22:28 | |
| menn0 | thumper, so the stateservinginfo doc was only being created via the functionality for dealing with legacy envs? nice. | 22:29 |
| thumper | yeah | 22:29 |
| thumper | found that by failing tests when I removed it | 22:29 |
| thumper | local bootstraps fine | 22:30 |
| menn0 | thumper, cool | 22:31 |
| menn0 | thumper, I've replied to your review comments | 22:39 |
| * thumper nods | 22:39 | |
| thumper | menn0: that comment makes no sense.. | 22:43 |
| thumper | This is capturing the upgradedToVersion field from the agent's config as the agent starts up - i.e. the version we've just upgraded from | 22:43 |
| thumper | I read that as: | 22:43 |
| thumper | 'upgradedToVersion' is the version we have just upgraded from | 22:43 |
| thumper | which seems arse backwards | 22:43 |
| menn0 | it is | 22:43 |
| menn0 | upgradeToVersion is the version for which upgrades have successfully run | 22:44 |
| menn0 | it gets updated to version.Current once upgrade steps have successfully run | 22:44 |
| menn0 | but when a machine agent starts up, before the upgrade steps have run | 22:45 |
| menn0 | it'll be set to the previous agent version | 22:45 |
| thumper | ok, that kinda makes sense | 22:45 |
| menn0 | I didn't name this stuff | 22:45 |
| menn0 | it's before my time | 22:45 |
| menn0 | thumper, I'm so used to this part of the code now that it doesn't even seem strange to me anymore | 22:47 |
| menn0 | but I do remember finding it confusing initially | 22:47 |
| * thumper nods | 22:47 | |
| thumper | wallyworld: I took a very quick look at bug 1359333 (blocking landing) | 22:59 |
| mup | Bug #1359333: win client thinks the cloud-init log is on the c drive <ci> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1359333> | 22:59 |
| thumper | but I can't see how the diffs cause the failure | 22:59 |
| wallyworld | thumper: ok, i'm about to go into a meeting, but can look after that | 22:59 |
| * thumper has standup now too | 23:00 | |
| davecheney | wallyworld: https://github.com/juju/juju/commit/3a8f263f8c2977241fc39450e9fb47b4d56fc975 | 23:24 |
| davecheney | this was the commit that broke CI | 23:24 |
| davecheney | please revert it | 23:25 |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1359333 | 23:25 |
| mup | Bug #1359333: win client thinks the cloud-init log is on the c drive <ci> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1359333> | 23:25 |
| wallyworld | davecheney: hi, just finished a call, looking now | 23:31 |
| davecheney | wallyworld: the bug is it uses paths.DataDir which uses version.Current | 23:32 |
| davecheney | and version.Current, when you're using windows tools, will using windows tools | 23:32 |
| davecheney | this is reason number 9001 why version.Current is lethal | 23:32 |
| wallyworld | davecheney: do these guys doing windows develppment event test their stuff on windows before landing? | 23:32 |
| davecheney | wallyworld: that's between them and the person that reviewed their code | 23:33 |
| wallyworld | well i can understand how stuff gets through a review, but not to test on the platform you are developing for.... | 23:34 |
| thumper | holy cow: 44 changed files with 1,954 additions and 360 deletions. | 23:38 |
| thumper | wallyworld: I don't think they are really testing it manually | 23:38 |
| thumper | that should really be raised as a concern | 23:38 |
| wallyworld | YES | 23:38 |
| thumper | alexisb: ^^ | 23:38 |
| wallyworld | thumper: we'll discuss tonight at team leads' meeting | 23:39 |
| thumper | ack | 23:39 |
| wallyworld | thumper: davecheney: so are we 100% sure that https://github.com/juju/juju/pull/189 is the bad one? i'll just hit the Revert button if so | 23:40 |
| davecheney | wallyworld: not 100% sure | 23:40 |
| davecheney | what's the worse that can happen ? | 23:40 |
| thumper | wallyworld: can't say 100%, but indications are high | 23:40 |
| davecheney | we just re apply iy | 23:40 |
| wallyworld | ok | 23:41 |
| davecheney | man version.Current is a timb bomb | 23:42 |
| alexisb | thumper, wallyworld, davecheney: sorry I have to run, but I added a note about the windows workload reviews to the leads agenda so we wont forget | 23:42 |
| wallyworld | +1, and not just reviews, actual proper testing and due dilligence prior to landing | 23:42 |
| wallyworld | davecheney: thumper: reverted, let's now wait to see if CI is happy again | 23:46 |
| thumper | davecheney: did you mark it as fixes the bug so ci is unblocked? | 23:47 |
| thumper | sorry davecheney | 23:47 |
| thumper | wallyworld: ??^^ | 23:47 |
| davecheney | thumper: thats ok thumper | 23:47 |
| davecheney | wallyworld: how long do we have to wait ? | 23:47 |
| wallyworld | thumper: i didn't because the revert didn;t go through ci | 23:48 |
| wallyworld | but i can mark the bug as resolved | 23:48 |
| wallyworld | pending CI running it's tests | 23:48 |
| wallyworld | we can reopen if needed | 23:48 |
| * thumper wonders if it should have... | 23:48 | |
| davecheney | \o/, one down, one to go | 23:49 |
| wallyworld | thumper: maybe it should have, but i'll pull trunk now and run the tests | 23:50 |
| wallyworld | WTF | 23:54 |
| wallyworld | FAIL: dependencies_test.go:42: dependenciesTest.TestGodepsIsRight | 23:54 |
| wallyworld | dependencies_test.go:77: | 23:54 |
| wallyworld | ... | 23:54 |
| wallyworld | dependencies_test.go:70: | 23:54 |
| wallyworld | c.Fatal(string(out)) | 23:54 |
| wallyworld | ... Error: godeps: no version control system found for "/usr/lib/go/src/pkg/bufio" | 23:54 |
| wallyworld | godeps: no version control system found for "/usr/lib/go/src/pkg/bytes" | 23:54 |
| wallyworld | thumper: davecheney: you guys seen ^^^^ as a result of PR 495? | 23:55 |
| thumper | huh? nope | 23:56 |
| wallyworld | sigh | 23:56 |
| thumper | I do think that a test for godeps isn't right | 23:57 |
| thumper | and the make rules I added were better | 23:57 |
| thumper | but .. | 23:57 |
| * thumper shrugs | 23:57 | |
| * wallyworld will reply to list | 23:57 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!