[00:00] yeah, I think two methods could work [00:00] my suggested name sucks :) [00:01] thumper: how can i test the command behind a feature flag? [00:01] thumper: how do i set feture flag for tests*? [00:01] anastasiamac: set the feature flag in the test [00:01] anastasiamac: I have something in my current branch [00:01] anastasiamac: that makes it trivial [00:01] anastasiamac: can you wait? [00:02] where current means hopefully by the eod [00:02] thumper: i can wait for the final land, but I need a way while developing now... [00:02] thumper: i'll look at what's currently done.. [00:02] anastasiamac: look at the actions code [00:03] thumper: unfortunately, the test is not mine, m just adding to it... [00:03] thumper: thnx! [00:05] anastasiamac: cmd/juju/action/action.go :: const FeatureFlag string = "action" [00:05] anastasiamac: and then just grep usages of FeatureFlag in cmd/juju/action/... [00:06] jw4: yep. i got this far. but it's never set in any of the tests.... [00:07] anastasiamac: ah; gimme a sec [00:07] jw4: btw, my branch does change that... hopefully it'll land by EOD [00:07] if all goes well [00:09] thumper: right. i have all the per-env workers starter under the envworkermanager [00:09] thumper: but there's no way it'll work for a non-state server env just yet [00:09] yep [00:09] anastasiamac: cmd/juju/main_test.go [00:09] menn0: getting there :) [00:09] thumper: yep :) [00:10] thumper: the remaining bits aren't hard, but I do need to change the envworkermanager a little [00:10] thumper: doing that first in a separate branch [00:10] anastasiamac: setFeatureFlags("...") [00:10] jw4: AHHH! AWESOME :D [00:10] jw4: thnx! [00:11] anastasiamac: yw - I'm looking forward to seeing thumpers branch too :) [00:14] jw4: u r removing "action" for one of the runs... mind if I'll modify it ot be a bit more genric to remove any features that are behind the flags? [00:15] anastasiamac: not at all - please do [00:15] jw4: gr8 :D [00:27] menn0: branch done, just waiting for the factory one to land [00:27] before proposing [00:27] * thumper pops the stack again [00:28] thumper: kk [00:34] thumper: here's a little change to envWorkerManager that I need to get things working correctly in the machine agent [00:34] http://reviews.vapour.ws/r/785/ [00:37] menn0: http://reviews.vapour.ws/r/786/ [00:41] jw4: m thinkn http://pastebin.ubuntu.com/9812212/ [00:42] anastasiamac: that looks good to me [00:42] jw4: excellent! i'll keep it for now until thumper's branch lands :P [00:43] coolio [00:43] :) [00:44] wallyworld_: got time for a chat about leadership settings before the standup? [00:44] katco: maybe, i'm in the middle or trawling log files for a potential 1.21 issue, give me a few minutes and i'll ping you [00:45] wallyworld_: np at all, whenever you're ready [00:46] * thumper has popped back up to the start of the day [00:46] just one branch on the stack now [00:46] thumper: looking [00:53] ah poo [00:53] can't wrap the facade registration in the init method because the init methods are called before any other code, like mocking out the environment [00:53] so we have to intercept calls [00:54] bah humbug === kadams54-away is now known as kadams54 [00:57] thumper: review done [00:57] looks good === kadams54 is now known as kadams54-away [01:07] sometimes embedding feels like being back with html and iframes [01:17] * thumper pushes two items on to the stack [01:18] thumper: can we have a hangout? I have some stuff to show you. [01:18] menn0: give me five minutes? [01:18] thumper: sure [01:23] menn0: now? [01:23] hangout again? [01:23] wow, exactly five minutes [01:23] amigood or amigood? [01:24] menn0, axw, wallyworld_: https://github.com/juju/utils/pull/106 [01:24] thumper: we're in standup, [01:24] kk [01:27] thumper: sorry... I missed your message [01:27] thumper: hangout now? [01:28] I'm there [01:33] * perrito666 remaps his brain for go instead of python where said map was still missing [01:37] it is amazin how python finds ways to crawl back === kadams54-away is now known as kadams54 === TheRealMue is now known as TheMue [02:24] * thumper headdesks [02:24] why isn't this working... [02:25] ah fark... [02:28] WTF? why is it looking for facade version 0? [02:29] thumper: version 0 is for facades that existed before versioning was done... [02:29] but I have registered it with version 1 [02:29] but the apiserver is doing a lookup for 0 [02:29] version 1 is returned in the login results [02:30] thumper: m sure it is doing what u asked it to :D [02:32] thumper: if u r in the tests, i saw hardcoding to 0... [02:32] thumper: testing/apicaller.go [02:33] thumper: in juju/juju/api/base [02:33] caller.BestFacadeVersion is returnig 0 [02:33] thumper: yep :( [02:33] WTF? [02:34] damn,... this code is terrible [02:36] I needed to add to the map in api/facadeversions.go === kadams54 is now known as kadams54-away [02:39] morning o/ [02:40] dimitern: I see your morning and raise you a gnight [02:40] perrito666, good night to you then ;) [03:04] that moment when you think you've finished a review, only to see you're on page 1 of 3 [03:11] axw: m glad that fun is equally shared within the team :D [03:12] for some value of fun === kadams54-away is now known as kadams54 [03:40] thumper or axw: an easy review: http://reviews.vapour.ws/r/788/ [03:41] menn0: a not so easy review http://reviews.vapour.ws/r/789/ [03:41] thumper: that seems like an unfair trade :) [03:42] :) [03:43] thumper: looking :) [03:58] thumper: done. a thing of beauty :) [04:09] menn0: fixed and submitted for merge [05:14] wallyworld: I'm just going to change the diskformatter apiserver to filter out unattached block devices [05:14] should've done that in the first place [05:14] ok [05:15] wallyworld: published my reply so you can see my comments while I'm doing that [05:15] looking [05:19] axw: disagree about the singular comment. singular tends to apply for Set operations; but we also have ListKeys(), AddKeys(), MachinesWithTransientErrors(), WatchEnvironMachines() etc [05:19] the plural reflects that we get multiple results [05:20] wallyworld: you can argue either way. in apiserver/provisioner we have Machine, DistributionGroup, ProvisioningInfo [05:21] Machine() get a single machine only [05:21] for a single given tag [05:21] whereas here we are getting multiple BlockDevices for many tags [05:22] wallyworld: uniter AssignedMachine, GetOwnerTag ... these take params.Entities [05:22] whatever, I can change it [05:24] if that's ok it would be good. AssignedMachine is a little unfortunate but i can see why it was done - it gets the single AssignedMachine for each of the specified units [05:25] axw: when done, can you ping me as i'd like to discuss filesystem providers etc [05:29] wallyworld: just testing live, won't be long [05:29] sure [05:41] le sigh, now I've broken something [05:42] wallyworld: let's chat, I'll fix this later [05:42] ok === kadams54 is now known as kadams54-away [06:55] axw: i imagine we will be adding a new hook context value for the storage attached hook - "storageInstanceId". this will then be used in storage-get. agree? [06:55] wallyworld: yes [06:56] rightio [06:56] wallyworld: although, I've been wondering whether it shouldn't just be storage-changed; collapse multiple attachments into one event [06:56] in which case there would be no ID [06:57] and instead we'd need a storage-list [06:57] which we may well want anyway [06:57] hmmm, that might work [06:57] since the spec calls for storage-get, perhaps we should still do that [06:57] and storage-attached [06:58] wallyworld: we would need storage-get anyway, but it may need a parameter [06:58] i could make storage-get take a list [06:58] well, it would need the storageInstanceId parameter [06:58] i could make that a slice of ids [06:58] i guess [06:59] i assume hook tool parameters can be slices, never written one before [06:59] wallyworld: they're just commands, they can take whatever you want them to [07:00] wasn't sure about the marshalling implications [07:00] of passing around the parameters [07:01] oh i see, there's a Context [07:01] never written a hook or hook command before :-) === cmars` is now known as cmars [10:04] voidspace, axw, wallyworld, others? PTAL http://reviews.vapour.ws/r/790/ [10:04] dimitern: core meeting? [10:04] TheMue, omw [10:05] dimitern: oops, got distracted omw [10:11] axw, perrito666, wallyworld, team meeting? [10:29] dimitern: TheMue: are we waiting for 11am for our standup or doing it now? [10:29] voidspace: dimitern: np with doing it now [10:30] voidspace, TheMue, just give me 3 mins and I'll be there [10:30] dimitern: ok [10:30] ok [10:34] voidspace, i'm in the hangout [10:35] omw [10:41] morning [10:48] perrito666: o/ [11:00] voidspace, I've reviewed your NetworkInterfaces() implementation for MAAS - it's awesome, please propose it :) [11:36] * dimitern steps out for a while [12:26] TheMue: thanks for the review [12:27] TheMue: listConnectedMacs calls the MAAS API list_connected_macs [12:27] voidspace: yw, only minor notes [12:27] TheMue: so I don't think the suggested name change is actually helpful [12:27] TheMue: but a comment is a good idea [12:27] voidspace: I know, but inside a code I like to use its conventions instead of the one of the API it calls ;) [12:28] TheMue: well, the name should make it's clear what its doing - and I don't think it's unclear in usage or naming [12:28] TheMue: and the parallel with the underlying API is useful for understanding [12:28] I'll happily add the comment though [12:28] doing it now [12:28] voidspace: also sometimes more explicit naming is helpful for the poor guy maintaining this code in three years :D [12:33] TheMue: comment pushed, should be clear now http://reviews.vapour.ws/r/791/diff/# [12:41] I somehow managed to crash my machine [12:41] dropped the keyboard, knocking the usb hub, and whoops... [12:41] wow, that is amazing [12:42] perrito666: yeah, not sure what happened :-) [12:44] I would love to see the bug report :p [12:46] mfoord: https://bugs.kde.org/show_bug.cgi?id=108312 [12:46] it used to be bundled with a picture of the ferrent [12:46] ferret* [12:46] Hah [12:46] nice [12:47] https://bugsfiles.kde.org/attachment.cgi?id=11622 [12:58] mfoord, you've got a review [12:58] mfoord, and thanks for reviewing mine - btw can you clarify a bit about replaceContainerConfig? [12:59] mfoord, ah, sorry I got you [13:00] mfoord, so I didn't want to couple replaceContainerConfig too much with the networking config, as like this it's also useful for storage config [13:01] dimitern: ah, ok [13:01] dimitern: at least instead of a list of lines it could take a string and do the split itself [13:01] dimitern: have to call split manually before every call (all one of them, right?) seems pointless [13:01] dimitern: not tying it to network is fine though [13:01] mfoord, fair point [13:01] mfoord, will do the split internally [13:02] TheMue: dimitern: thanks for the reviews guys [13:02] dimitern: I'm still looking through that PR [13:02] dimitern: but going on lunch now, will return to it [13:02] mfoord: yw [13:06] mfoord, sure, enjoy! [14:11] dimitern: would you mind taking a look at https://github.com/TheMue/juju/tree/networking-interfaces? [14:11] dimitern: it not yet has merged michaels latest changes [14:11] dimitern: but so far it compiles and tests fine [14:20] TheMue, sure, I'll have a look in a bit [14:23] dimitern: thx [16:01] ericsnow: standup? [16:02] wwitzel3: coming [16:03] natefinch, master (1.23) has a regression in the last commit. I reported bug 1413652 [16:03] Bug #1413652: TestNetworkInterfaces fails on ppv64el unit tests

[16:05] dimitern, you still around? [16:07] natefinch: ? === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1413424 1413424 [16:17] alexisb, yes [16:18] dimitern, I was just wondering if you had insight on bug 14135652 mentioned above [16:18] TheMue, mfoord, I've updated the PR - can I get an LGTM please? :) [16:18] alexisb: it's just using DeepEquals where it shouldn't [16:18] dimitern: looking [16:18] alexisb, hmm.. I might have, let me have a look [16:19] alexisb, mgz, oh this looks like a map ordering ppc64 issue [16:20] dimitern: yup [16:20] dimitern: yep, reading [16:21] mfoord, yeah - definitely; sorry I should've thought of that - can you propose a fix for that bug? [16:22] dimitern: I meant I was reading your PR... [16:22] mfoord, basically the results should not rely on map traversal order but be sorted (e.g. by DeviceIndex) [16:22] dimitern: ah, is this my fault? [16:22] dimitern: ok, easy [16:22] mfoord, i'm afraid :) [16:22] dimitern: I'll do it now [16:22] mfoord, cheers! [16:24] mgz, thanks for bringing this up so quickly btw [16:24] sinzui was on the ball [16:27] yes sinzui thank you! [16:27] thank you sinzui [16:27] dimitern: do we have anything in utils for sorting structs by field name? [16:28] dimitern: or should I wrap them? [16:28] mfoord, I don't believe so [16:28] dimitern: ok [16:28] mfoord, where is voidspace? [16:28] alexisb: heh, not sure [16:28] mfoord, if you check network.SortAddresses - it should be similar [16:28] mfoord: I think what we did last time was just add the sort methods [16:28] like dimitern points at there [16:29] alexisb: I hard crashed a while back and I guess "voidspace" was still logged in when I resurfaced... [16:29] TheMue, sorry, I'm looking at your branch now - got distracted [16:29] mgz: dimitern: we're adding sort methods to production structs just to be able to test them? [16:30] mgz: dimitern: ah, SortAddresses is used in production too [16:30] mfoord, :) well isn't it nice to have a deterministic result? [16:30] mfoord: yeah, I prefer having any api visible stuff in stable order [16:30] mfoord, so not just for testing [16:31] for backend things, just sorting in the test or doing contentsequals instead of deepequals is fine [16:31] mgz: dimitern: ah, SortAddresses is used in production too [16:31] hah, oops already said that [16:31] :) [16:32] mgz: this isn't API visible stuff - I think this is just arbitrarily ordered test input data [16:32] I *think*, checking [16:32] dimitern: np [16:32] TheMue, first look - environs.Networking should also include SupportsAddressAllocation [16:33] dimitern: thought about, but on the other hand it's a capability [16:33] dimitern: can move it [16:34] TheMue, well, SupportNetworks was also a capability IIRC [16:34] dimitern: got me :D [16:35] TheMue, :) so let's try moving SAA() into the interface and make it a regular method [16:36] dimitern: will do so [16:36] TheMue, cheers === enterprisedc_ is now known as enterprisedc [16:38] TheMue, also, for ec2 and MAAS which already supports networking ISTM we should do the SupportsNetworking once somewhere - like in SetUpTest or e.g. setUpInstanceWithDefaultVpc [16:38] TheMue, to save a few lines and avoid calling it in each test [16:39] dimitern: it would be relatively easy to write a generic version of that sort taking a field name and using reflection... [16:40] mfoord, I agree, would you prefer doing this in utils and then proposing the fix? [16:40] dimitern: heh, that will take me longer [16:40] dimitern: I've nearly finished the "standard" fix [16:40] mfoord, :) exactly my point [16:40] dimitern: I'd be interested in doing the general one afterwards [16:41] dimitern: I would much *prefer* doing a general one [16:41] mfoord, but we should add a generic sort like this to utils nevertheless - after that [16:41] mfoord, +1 [16:41] dimitern: cool, sounds like fun :-) [16:42] dimitern: yes, that's my planning. only wanted to show the mechanism to check if an env supports networking and the return of the combined interface [16:43] TheMue, it looks solid enough for proposing - minus those few suggestions [16:43] TheMue, and I like how many lines get removed [16:43] dimitern: fine, will change those then and then do the proposal [16:44] dimitern: yeah, that's one of the nice features, remove code [16:44] TheMue, cheers! the cmd/juju/action/queue.go seems it doesn't belong there though [16:44] TheMue, I'll review it more carefully when you propose it [16:45] dimitern: ouch, added it by accident :/ [16:48] TheMue, np :) [16:56] dimitern: [16:57] dimitern: http://reviews.vapour.ws/r/792/ [16:57] mfoord, looking [17:00] mfoord, reviewed [17:01] dimitern: see my last commit message... [17:01] dimitern: https://github.com/juju/juju/pull/1470 [17:01] dimitern: and thanks :-) [17:02] mfoord, :D [17:02] mfoord, thank you for the quick fix [17:03] mfoord, can I get LGTM on mine now please? ;) [17:03] dimitern: going back to reading it [17:03] mfoord, cheers [17:09] dimitern: so when updating the config we overwrite the file in place? [17:10] dimitern: a common pattern is to write the new file as a temp one and then only move it into place when writing is complete [17:10] dimitern: avoids creating partial files / invalid files due to a bug [17:10] dimitern: probably not an issue for config changing though, because if we crashed we wouldn't use the config file anyway? [17:12] mfoord, fair point, and I was itching to use AtomicWriteFile, but since none of the other config updating calls do it I decided not to for now [17:12] dimitern: ok [17:12] dimitern: if you don't think it's worth it then just drop the issue I created [17:15] dimitern: ah wait, you write to it in one go at the end anyway - you're "writing into" a bytes buffer anyway [17:15] dimitern: and then you seek, truncate and write [17:15] seems like a lot of boilerplate just to write a file... ah well. [17:18] dimitern: there's never a need for duplicate prefixes in lxc? [17:20] mfoord, ok, I'll look into changing it to use AtomicWriteFile instead [17:21] mfoord, there are valid cases for duplicate prefixes - e.g. each lxc.network.type defines a new NIC config [17:21] dimitern: ah, but you'd want the line multiple times in the input lines then? [17:22] dimitern: it's just that popping it from the parsedLines values dictmeans multiple replacements need multiple entries [17:22] sounds like that's desirable though [17:22] "lxc.network.type = foo" in the input line will only replace the first occurence [17:24] heh, testing templates makes for long tests [17:24] dimitern: LGTM [17:25] mfoord, yes - I tried to explain the way it works, but I'm bad at explaining complex things :) [17:25] mfoord, thanks! [17:25] "occurrences of any setting in lines will be replaced with the value of that setting." [17:26] hmm, is that true? Let me read the code again. [17:27] parsedLines is a map of prefixes to new values to use [17:28] once a prefix has been encountered, the first value from parsedLines[prefix] is used - and the used value is *removed* [17:28] mfoord, it actually got more difficult to explain with a string rather than []string lines :) [17:28] mfoord, yes [17:28] so next time it is encountered the prefix *won't* be replacedd with the new value [17:29] so it isn't "any setting in lines" - it's the first occurence of that setting is used. If the same prefix is specified multiple times they are used in order. [17:29] mfoord, no, so if "lxc.foo" > []string{"bar","baz"} and you have lxc.foo=1\nlxc.foo=2\lxc.foo=3\n it becomes lxc.foo=bar\nlxc.foo=baz\nlxc.foo=3\n [17:30] right, but if I have "lxc.foo" > []string{"bar"} [17:30] the documentation implies that "any occurence" of "lxc.foo" will become bar. [17:30] mfoord, then it will be lxc.foo=bar\nlxc.foo=2\nlxc.foo=3\n [17:30] Whereas it's actually only the first. [17:30] mfoord, right, right - I'll change this [17:30] "occurrences of any setting" [17:31] dimitern: it's fiddly to explain as what it is doing is fiddly [17:31] mfoord, how would you explain it now that you know what the code does? [17:31] but "occurrences of any setting" implies to me multiple replacements from a single value [17:31] hehe [17:31] :) [17:31] mfoord, maybe just with a few examples? [17:32] you've pointed people to the tests and there are some examples there [17:32] so I don't think more examples are needed in the docstring [17:32] just maybe a wording tweak for that one line [17:32] I'm thinking [17:33] Then the occurrence of a setting in a line of the config file will be replaced by values from newConfig. Values in newConfig are only used once (in the order provided), so multiple replacements must be supplied as multiple input values in newConfig. [17:34] Replacing "Then any..." [17:34] It's not so concise but a bit more precise I think. [17:34] and I need more coffee [17:35] dimitern: mgz: the bug fix for 1413652 landed so I changed the issue to fix committed [17:36] mfoord: ace [17:36] mfoord, nice, I'll take it :) [17:36] mfoord, thanks [17:36] mgz: ace is such an eighties word :-) [17:36] it's when I was born :) [17:36] :-) [17:37] my Dad still uses it because we used it when we were kids [17:37] he thinks it's still cool to be ace [17:37] and I guess he's right... [17:37] * mfoord goes on a coffee hunt... [17:45] ericsnow: ok, grabbing some food then we can take a look at that review [17:45] wwitzel3: great === kadams54 is now known as kadams54-away [18:10] perrito666, ping [18:34] hazmat: pong [18:35] perrito666, just trying to understand some of the backups api [18:35] perrito666, afaics info method doesn't have a purpose, given all content is in list [18:37] hazmat: you will have to talk to ericsnow about it, I just wrote restore, there is little left of th backup logic I once did. [18:37] it might be redundant though, since backup+restore lacks a spec it was mostly played "by ear" [18:38] perrito666, ok.. i think i got most of it from the code, i'm just looking at trunk [18:39] hazmat: hacking something into backups? [18:39] perrito666, exposing all the facades via jujuclient atm [18:40] hazmat, perrito666: yeah, Info is effectively just a special case List (where you don't have to look up the one you care about in the result) [18:40] and exercising all the apis while i'm at it. [18:41] sort of wishing that agent version was in login result [18:41] its a bit hard to specialize some of the /environment/:uuid/backups|charms etc urls without knowing the version [18:42] hazmat: keep in mind that the backups API methods related to restore probably should be avoided (once they land) [18:42] ericsnow: headed in to moonstone [18:43] hazmat: they have to work together and require some set up (the restore command will cover all that) [18:43] ericsnow, cause their not stable? [18:43] cool [18:43] its just upload & restore i would hope, but fair enough [18:44] hazmat: each of the restore-related methods makes assumptions (e.g. that bootstrap has already happened in a new env) [18:44] hazmat: restore is abig piece of twisted machinery and its landed in small bits since no one seems to be able to review it as a whole so I am entering the bits as they are approved [18:45] hazmat, perrito666: I suppose it could make sense to roll that logic out of the command and into the API client, but bindings other than our API client would have to know those details to incorporate them [18:45] bootstrap already seems reasonable, i'll bake what i can into my functional tests. [18:46] perrito666: perhaps you could give hazmat a link to the cmd and API client code so he can see what's going on [18:47] i don't see restore in trunk looking at api server source. [18:47] hazmat, perrito666: really it would be nice if restore could be handled on the server side by a single API call, but I'm not sure that's feasible (or worth it) [18:48] hazmat: http://reviews.vapour.ws/r/732/ (just the API server part) [18:48] ericsnow, thanks [18:50] so three calls prep/restore/finish [18:51] hazmat: yup, finish is not really required, you see prep sets the server in a status that prevents anything el se to be done (to avoid data loss from whatever you asked the server to do between deciding to restore and restore actually taking effect) [18:51] restore does well, restore [18:52] and finish just finishes ok if restore finished properly (restore will kill the server so your connection will be droped) [18:53] got it, thanks [18:54] somehow each time I explain this part of restore I think of willie coyote cutting the branch he is standing on [19:05] g'night all [20:07] * perrito666 listens to the afternoon news on the radio and wonders why did he decide to do that in the first place [20:12] waigani, cherylj, menn0: morning [20:13] oops [20:13] mwhudson: ? [20:14] forgot stand up was now :) [20:36] Good god, bank websites blow [20:41] natefinch: mine works well although the bank might have been forbidden to do certain operations by the govt bc they where helping in money laundring schemes [20:42]