/srv/irclogs.ubuntu.com/2016/08/09/#juju-dev.txt

redirperrito666: which bug?00:03
rediraxw: go a minute?00:23
redirgot even ^00:24
mupBug #1611159 opened: model not successfully destroyed, and error on "juju list-models" <oil> <oil-2.0> <juju-core:New> <https://launchpad.net/bugs/1611159>00:46
wallyworldredir: did you want to catch up?00:49
wallyworldor you all good?00:49
redirwallyworld: sure, gimme 1 minute00:49
redirk00:51
redirstandup?00:51
redirwallyworld: ^00:51
wallyworldok00:51
axwredir: sorry, was afk01:11
redirnp just off HO with wallyworld, going EoD but will prolly hit you up tomorrow.01:29
* redir goes EoD01:33
=== natefinch-afk is now known as natefinch
natefinchaxw: let me know when/if you'd like to spend a little time discussing the jsonschema stuff... otherwise, Í'll get to writing that proposal for it that I was supposed to do earlier.02:05
axwnatefinch: happy to chat whenever, I think having a proposal first would be best though - something concrete we can discuss and mould02:07
natefinchaxw: cool, will whip something up.02:08
natefinchweird, somehow chrome is linking zoom size to domain... if I have two different chrome windows open - if they're the same domain, they both increase or decrease in size. If they're different urls they change independently.02:28
natefinchwhich is super annoying when I want to have godoc open on two different monitors with vastly different pixel densities02:29
* natefinch switches one window to gowalker.org ... oh the indignity02:31
axwanastasiamac: I have added this change to the PR, as it was failing with mongo 2.4: https://github.com/juju/juju/pull/5877/commits/ac97bffd0261f06df93255bed0a48ce35d1a5eb102:56
axwanastasiamac: tested with mongo 2.4 and 2.6 (2.6 works with and without the change)02:56
axw2.4 works only with the change02:56
anastasiamacaxw: 2.4 does like "or" :( ..?..02:57
axwanastasiamac: not really sure what it doesn't like, seemed kosher to me02:58
axwanastasiamac: but they're bot happy with $ne even if hte field doesn't exist02:58
anastasiamacexcellent \o/ lgtm02:59
wallyworldmenn0: one thing that i miss when using juju from a snap is tab completion :-)03:33
menn0wallyworld: yeah true. I guess there might be some kind of workaround where juju completion workers for *.juju too but that's a bit hacky03:33
menn0s/workers/works/03:34
wallyworldi'm sure we can fix it somehow03:34
mupBug #1580391 changed: juju/errors tests do not pass with go 1.6 <tech-debt> <unit-tests> <juju-core:Fix Released by dave-cheney> <https://launchpad.net/bugs/1580391>03:56
axwanastasiamac: PTAL at http://reviews.vapour.ws/r/5392/ when you have a moment03:59
axwanastasiamac: i.e. please see my replies03:59
* axw goes for lunch03:59
anastasiamacwallyworld: could u do a quick ho in a-team call?04:08
wallyworldok, give me a minute04:08
anastasiamacwallyworld: k. ping u r available04:09
anastasiamacwhen*04:12
natefinchmy proposal for conversion to jsonschema for anyone interested: https://docs.google.com/document/d/137ejUl8qRAU98F4eg9jfO9L90fkkvRZ5aGINmaxKlM0/edit#04:13
wallyworldanastasiamac: there now04:14
=== tasdomas` is now known as tasdomas
natefinchaxw: I'm a little stuck on the actual implementation of getting the non-jsonschema values into useful places in our code... without terrible hacks.  Any ideas you have there are welcome.  I'm going to go to bed.. will check my email overnight, so when william gets up, you guys figure out a time to talk, and I'll get up and talk.04:53
natefinchaxw: unless you think it would be useful to talk now for a bit, without william04:53
axwnatefinch: I need to think about it some more first I think04:54
axw... think think04:54
natefinchaxw: no problem. I've gone around and around on it.  I welcome more brains being applied to the problem.04:54
fwereadeaxw, menn0, jam: shall we talk schemas?06:52
axwfwereade: happy to, but shouldn't we wait for natefinch?06:52
fwereadeaxw, perhaps, but I'm not sure when he'll arrive; and several of us look likely to be here; and I got the impression that you were pretty happy with the proposal in the doc06:54
axwfwereade: okey dokey, I can chat with natefinch about it tomorrow if need be06:56
fwereadeaxw, so, well, *I'm* happy with the proposal in general, and have no attachment to juju/gojsonschema if a different library gives us something we need07:01
axwfwereade: agreed, as long as we end up with one or the other07:01
axw(and not both)07:01
fwereadeaxw, right, yes, we should replace the action bits if we switch07:02
axwfwereade: do you have an opinion on whether we should retain the EnvVar bits?07:03
axwor Group?07:03
axwI'm *fairly* sure Group is redundant now07:03
axwanything that's "juju defined" shouldn't really be in the model config anyway07:04
* axw just waits for doc comments to roll in07:05
fwereadeaxw, I'm very happy to drop group07:05
fwereadeaxw, and I am a bit snooty about the env var bits07:05
fwereadeaxw, do you think we can cast them as interactive-only? in which add-credential is considered interactive?07:06
axwfwereade: that's all they would be used for07:06
fwereadeaxw, it's the "magically grab them from the env" stuff that I always hated07:06
axwfwereade: autoload-credentials and add-credential07:06
fwereadeaxw, yeah07:07
axwfwereade: yeah, that should definitely not happen post-add07:07
axwfwereade: I guess I can live with it. but it feels kinda awkward that there will be some credential detection methods that just won't work, like looking for files07:08
fwereadeaxw, (if I were to shoot for the moon I'd prefer a schema that didn't allow defaults, tbh, it's just another way to handwave the difference between ingestion and validation)07:08
fwereadeaxw, well, we *could* define file paths as well...07:08
fwereadeaxw, not that I really want to07:09
fwereadeaxw, but the *important* thing in my mind is not to have the *validation* bits go charging off through the env, filesystem, network, whatever, just to find some value to wedge in and make things look right07:10
* axw nods07:10
axwfwereade: these defaults should only apply when creating. providers can/should take care of adding defaults as necessary on upgrade07:11
fwereadeaxw, yeah, which is why I'd like to keep default-insertion out of validation too, if we can07:12
fwereadeaxw, I just don't see a path that doesn't involve us writing our own schema thing07:12
axwfwereade: I mentioned this in London: https://github.com/juju/juju/blob/master/environs/interface.go#L12107:14
fwereadeaxw, we could just tell people not to specify json-schema defaults (ha), but if people *did* eschew them then it is just the same need as the env/file cases: if not specified, get a value <like this>07:14
axwfwereade: isn't it enough to just implement that in each provider, and stop modifying config in ValidateConfig?07:14
axwfwereade: ah I see what you mean, JSON-Schema validation will insert the defaults?07:14
fwereadeaxw, yeah07:14
fwereadeaxw, (regardless, I'm +1 on breaking up the ValidateConfig god-methods any way we can)07:16
axwfwereade: I would think we would just ignore the defaults post-creation07:16
axwsame for env vars07:16
fwereadeaxw, yeah -- that's easy if we can clearly separate them -- but I don't see how we can avoid default-insertion basically every time we validate a config07:17
anastasiamacaxw: wallyworld: menn0: any suggestions? or workaround for this? https://bugs.launchpad.net/juju-core/+bug/161115907:27
mupBug #1611159: model not successfully destroyed, and error on "juju list-models" <oil> <oil-2.0> <juju-core:Triaged> <https://launchpad.net/bugs/1611159>07:27
axwanastasiamac: nope, sorry07:30
anastasiamacaxw: tyvm!07:32
mupBug #1449044 changed: juju add-unit resets AWS security groups <add-unit> <ec2-provider> <security> <juju-core:Invalid> <https://launchpad.net/bugs/1449044>07:35
mupBug #1548564 changed: restart failed upgrade <juju-core:Invalid> <juju-core 1.25:Won't Fix> <https://launchpad.net/bugs/1548564>07:35
=== jamespag` is now known as jamespage
mupBug #1517499 changed: i/o timeout on bundle deployment <juju-core:Triaged> <https://launchpad.net/bugs/1517499>08:08
rogpeppeaxw: hiya08:28
axwrogpeppe: hey08:28
rogpeppeaxw: i just ran across mongotest.DialTimeout...08:28
rogpeppeaxw: it was causing a test to take 5 minutes08:28
rogpeppeaxw: is 5 minutes really necessary as the dial timeout?08:29
rogpeppeaxw: it seems like quite a long time...08:29
axwrogpeppe: per the comment, I think so. mostly for the unit test merge jobs, because the machines thrash quite a bit after starting up08:30
axwrogpeppe: why is the test taking 5 minutes? sounds like an error in the test?08:30
rogpeppeaxw: well, arguably it's a problem with state.State.Close08:30
rogpeppeaxw: or with the whole strategy used to manage mgo.Sessions in juju08:31
rogpeppeaxw: it's an interesting thing though08:31
rogpeppeaxw: the test that takes 5 minutes is apiserver.serverSuite.TestNewServerDoesNotAccessState08:31
rogpeppeaxw: well... 7/10 times it takes 0.08 seconds; the other times it takes 5 minutes08:32
rogpeppeaxw: it depends how far the state workers get before the state is shut down08:33
rogpeppeaxw: the problem is that the session is copied (with Session.Copy) whenever a mongo operation is performed08:33
rogpeppeaxw: and if the mongo is unavailable (as it is in this test - deliberately), the mongo operation will block until a connection can be made (or it times out)08:34
rogpeppeaxw: and while worker is trying to perform the operation, it cannot be stopped08:35
rogpeppeaxw: and the State cannot be closed until all the workers have been stopped08:35
mupBug # opened: 1611267, 1611269, 1611271, 1611273, 161127508:36
axwrogpeppe: hrm :(  I don't suppose we can tell when the workers are idle?08:36
rogpeppeaxw: how would that help?08:37
axwrogpeppe: don't close the proxy until nothing should be accessing it08:37
axwrogpeppe: which is... never? the transaction watcher will always be active won't it08:37
rogpeppeaxw: one mo, in a call08:40
babbageclunkfwereade: so I was bugging dimitern about api stuff yesterday evening, but it got late.08:43
babbageclunkfwereade: (ping)08:47
axwmgz: ping? would now be a good time for me to run some tests against finfolk?08:56
axwscratch that, will be back later08:59
fwereadebabbageclunk, hey, sorry09:01
fwereadebabbageclunk, I did do a little review this am, not sure if that's relevant?09:02
babbageclunkfwereade: no worries - just noticed the review, going through it now.09:02
babbageclunkfwereade: I'm not sure what you mean by "wrap the txn-building in a func that guarantees a sensible error"09:03
babbageclunkfwereade: You mean something other than onAbort?09:03
fwereadebabbageclunk, ah sorry09:03
fwereadebabbageclunk, there's run, which takes a `func(attempt int) error`09:04
babbageclunk(I mean, I understand the problem with letting ErrAborted get out, I'll fix that, just wondering if there's a deeper fix than chucking an onAbort around it.)09:04
fwereadebabbageclunk, which will get called again if the txn is aborted09:04
fwereadebabbageclunk, and if you structure the buildTxn func nicely, you have OAOO place where you check state, and can construct the associated assert at the same time09:05
rogpeppeaxw: yes, the transaction watcher (and the pinger) will always be active09:05
fwereadebabbageclunk, so when it fails it just gets called again and you never have to worry about, e.g., having two places to complain about non-dead machines that produce slightly different error messages09:06
babbageclunkfwereade: Ah, so run can recheck the preconditions each time to work out which one failed and abort with a nicer error?09:06
fwereadebabbageclunk, exactly09:06
fwereadebabbageclunk, https://github.com/juju/juju/wiki/mgo-txn-example09:07
babbageclunkfwereade: Right, so if I change Remove, MarkForRemoval and CompleteMachineRemovals to use those then places that add preconditions get a chance to return the right kind of error at the same time as they add the assertion.09:08
fwereadebabbageclunk, yes please :)09:08
babbageclunkfwereade: Ok, thanks, I'll do that.09:09
fwereadebabbageclunk, incidentally, if run failed enough you'll get ErrExcessiveContention out, which hsa text something "state changing too fast, try again soon"09:09
fwereadebabbageclunk, but what it almost invariably *actually* means is "some developer messed up the correspondence between state checks and txn asserts"09:10
babbageclunkfwereade: Ah, right.09:10
fwereadebabbageclunk, but that's generally easier to understand and fix than when an onAbort triggers in response to ops defined somewhere else, because that means you often just get the *wrong* error message09:11
babbageclunkfwereade: ok09:12
fwereadebabbageclunk, (because someone changed the ops and the client is still assuming there's only one way for the ops to trigger abort)09:12
rogpeppeaxw: i think the real solution is not to Copy the session on every database operation09:22
rogpeppeaxw: but that's a significant change09:23
=== mwhudson_ is now known as mwhudson
rogpeppea small juju-core code cleanup: https://github.com/juju/juju/pull/595411:03
rogpeppereviews appreciated. fwereade, you'll appreciate this change11:04
fwereaderogpeppe, very nice, LGTM with a trivial11:16
rogpeppefwereade: tvm11:16
rogpeppefwereade: that trivial is not *quite* so trivial. AFAICS there is no need for that remoteError value at all. it couldn't work even if the call did return an error. and i don't see why that code is invoking FacadeCall directly rather than using an api package.11:32
rogpeppefwereade: ha, that *is* the api package :)11:32
babbageclunkfwereade: Can you take a look at http://reviews.vapour.ws/r/5365/diff/# again? Everything's using buildTxn/run now, but I'm worried I might be doing too much in completeMachineRemovalsOps.11:38
natefinchaxw, fwereade: morning... anything you need to talk about?11:46
* babbageclunk goes for a run.11:53
wallyworldlooking for a review of a small CLI change for stakeholders to go into beta15 http://reviews.vapour.ws/r/5399/11:57
axwnatefinch: I think you're good to get deeper into it - not sure that there's anything to discuss. there's a few comments on the doc. I'm here for a bit but will be disappearing soon - will be more free to chat tomorrow if you're uncertain about anything11:59
axwmgz sinzui balloons: any of you about?12:01
natefinchaxw: no problem.  The comments seems clear, and I think we'll be good to go with it.12:03
axwnatefinch: did you look at replacing gojsonschema with the other one already?12:04
axwfor actions12:04
=== ivyyy_ is now known as ivyyy
axwI suspect it'd be trivial, I don't think we do anything particularly complicated there12:04
fwereadebabbageclunk, I think that's all fine12:39
fwereadebabbageclunk, completeMachineRemovalsOps is quite big but it doesn't seriously upset me12:40
babbageclunkfwereade: great! Thanks.13:06
sinzuiaxw: I am now about13:06
sinzuifwereade: babbageclunk What is the sig we send to jujud to stop and uninistall?13:23
babbageclunksinzui: I think it's a file that gets left in a directory somewhere, but I haven't found the bit of code yet.13:26
sinzuibabbageclunk: touch uninstall-agent, then SIGABRT?13:28
mupBug #1506225 changed: Failed bootstrap does not clean up failed environment w/o --force and error message is unhelpful <bootstrap> <destroy-environment> <jujuqa> <juju-core:Invalid> <juju-core 1.25:Won't Fix> <https://launchpad.net/bugs/1506225>13:31
mupBug #1611379 opened: api/backups: restore code needs improvement <juju-core:New> <https://launchpad.net/bugs/1611379>13:31
babbageclunksinzui: not sure about the signal, but that sounds plausible13:31
sinzuibabbageclunk: I am reading notes I left to myself more than a year ago. Plenty of time for juju to change13:33
babbageclunksinzui: True that. I'm just trying it out.13:35
wallyworldnatefinch: I think you're ocr? can i get a small review for a stakeholder request http://reviews.vapour.ws/r/5399/13:58
natefinchwallyworld: will do14:00
wallyworldta14:00
rick_h_fwereade: standup ping14:03
mupBug #1516150 changed: LXC containers getting HA VIP addresses after reboot <canonical-bootstack> <juju-reboot> <juju-core:Invalid by dimitern> <juju-core 1.25:Won't Fix> <https://launchpad.net/bugs/1516150>14:10
tvansteenburghHi everyone...in juju1 I could upload a local charm to the apiserver, then immediately deploy it via the Deploy api using the 'local:...' url returned by the upload. That doesn't work with juju2 - I get a 'charm not found' error from the api when I Deploy, and the Charms.List api doesn't show my charm. What am I missing?14:12
natefinchmgz, sinzui: for https://bugs.launchpad.net/bugs/1604474, can you leave up a windows VM that CI tried to deploy to?  I wasted a lot of time trying to get azure to start a windows machine yesterday14:14
mupBug #1604474: Juju 2.0-beta12  userdata execution fails on Windows <azure-provider> <ci> <juju2.0> <oil> <oil-2.0> <regression> <vpil> <windows> <juju-core:In Progress by natefinch> <https://launchpad.net/bugs/1604474>14:14
sinzuinatefinch: sure, we just add --keep-env to the job, or just run the test locally with the same flag14:15
babbageclunkHi tvansteenburgh - you can deploy a local charm just by specifying the charm directory in juju 2. Or do you want to upload it specifically?14:16
mgzthere's then a slight wiggle making sure you get the env details to actually inspect that machine14:16
mgzbut it's doable14:17
tvansteenburghbabbageclunk: yeah but only via the cli right? i'm using the api14:17
babbageclunktvansteenburgh: oh, I see. Sorry, not sure.14:18
babbageclunktvansteenburgh: Reading the code it sounds like what you're describing should still work. Posting to the /charms endpoint gives you a url back?14:26
tvansteenburghbabbageclunk: yeah, a local: url14:26
tvansteenburghbut then when i try to deploy using that url, i get charm not fourd14:26
tvansteenburghfound14:26
tvansteenburghand if i call the Charms.List api, my charm isn't in the list14:27
natefinchaxw: you're not actually around are you?14:27
sinzuinatefinch: I run the test scripts locally all the time. I think this will run the windows charms in azure and keep the env about after your send a control-c https://pastebin.canonical.com/162690/14:30
babbageclunktvansteenburgh: Sorry, I'm not familiar with that part of the code at all.14:30
rick_h_tvansteenburgh: katco and wallyworld were just talking about some work needed around that code14:30
rick_h_tvansteenburgh: I defer to katco on the exact work required/possible bug there but funny that you bring it up right as they're investigating that area of things14:31
mupBug #1516150 opened: LXC containers getting HA VIP addresses after reboot <canonical-bootstack> <juju-reboot> <juju-core:In Progress by dimitern> <juju-core 1.25:Won't Fix> <https://launchpad.net/bugs/1516150>14:31
mupBug #1611391 opened: model migration fails while removing from original controller <model-migration> <juju-core:Triaged> <https://launchpad.net/bugs/1611391>14:31
natefinchsinzui: thanks for the detailed example14:32
babbageclunkfwereade: around?14:33
katcotvansteenburgh: i'll ping you when i open that bug. from my discussions with wallyworld though, i would think that what you're doing should work (i.e. deploying from the api). there is likely something wrong with how we're caching charm archives14:33
fwereadebabbageclunk, heyhey14:33
babbageclunkfwereade: migrations!14:34
babbageclunkfwereade: I think I should migrate machineRemovals records?14:34
fwereadebabbageclunk, I think you should add a note saying they're not migrated because we won't migrate when we're in an unstable state14:35
babbageclunkfwereade: I *could* treat them like cleanups, but then migration would be blocked by the provider?14:35
fwereadebabbageclunk, and double-check with menn0 that "any entity in any state other than alive" == unstable (from migrations perspective)14:36
babbageclunkfwereade: Ok, so I should treat them like cleanups then - don't let migration happen while there are machineRemovals.14:36
babbageclunkfwereade: ok14:36
fwereadebabbageclunk, yeah -- and just sync up with menn0, I am pretty certain he's about to start on that side of it14:36
tvansteenburghkatco: thanks! really eager to get past this - trying to help a customer who is stuck here14:36
katcotvansteenburgh: ack. other than asap, what kind of timeframe would be ideal for you?14:37
babbageclunkfwereade: ok cool14:37
tvansteenburghkatco: yesterday? :)14:38
tvansteenburghkatco: i know you guys are swamped, whatever you can do14:38
katcotvansteenburgh: not a problem. in this timeline you'll have to wait 40y or so while i invent time travel, but the alternate-you will have it yesterday :)14:38
tvansteenburghhaha!14:39
niedbalskidimitern, re: 1610037, I just uploaded the cloud init logs for both container/host.14:39
balloonsjam, thanks for opening the conversation again about sharing configs for snappy14:39
natefinchkatco: ship it with one minor question14:54
katconatefinch: tal, ty14:54
katconatefinch: ah, yeah the reason i do that is so that the formatting is completely left to the errors package. basically for consistency14:55
katconatefinch: e.g. if the errors package ever changes its string formatting (or we use different errors), this would still format correctly in the log14:56
mupBug #1611404 opened: failed migration leaves model unkillable <model-migration> <juju-core:Triaged> <https://launchpad.net/bugs/1611404>15:01
natefinchkatco: It was a little confusing to me.  My first thought was that you were accidentally not saving the annotated error to a local variable.... took me a second to realize you were probably doing it on purpose to let it do the formatting for you.15:01
balloonsI believe my PR's are still invisible, so does someone mind having a look at https://github.com/juju/juju/pull/5956 today? It's upstreaming the snapcraft.yaml file15:17
dimiternniedbalski: thanks, I'll have a look15:29
dimiternniedbalski: /var/log/cloud-init.log from the container is not very useful I'm afraid - I'd like to see /var/log/cloud-init-output.log from the container please, and thanks for adding the machine's /v/l/c-i-o.log15:32
niedbalskidimitern, ok. cloud-init and cloud-init-output http://pastebin.ubuntu.com/22811404/15:34
dimiternniedbalski: is there nothing else in the container's /var/log/cloud-init-output.log? That's very strange..15:36
* rick_h_ grabs lunchables15:40
niedbalskidimitern, yep, it seems not complete. btw; http://pastebin.ubuntu.com/22811951/ this is the hypervisor.15:41
katconatefinch: hey you're OCR, can i get a quick ship it? +8L: http://reviews.vapour.ws/r/5401/15:50
mupBug #1611427 opened: Code which performs retries should consolidate on github.com/juju/retry <tech-debt> <juju-core:New> <https://launchpad.net/bugs/1611427>16:01
rick_h_katco: I'd feel free to self-review that one and just run with it16:07
katcorick_h_: k16:07
katcorick_h_: wondering... in the time it took you to type that message, could you have clicked "Ship It!" ;)16:07
mgzkatco: I'm happy to stamp a comment-only change :)16:08
rick_h_katco: yes, but then I'd not have said that in the future you should be able to self-review trivial such as that16:08
rick_h_katco: so figured I'd type that vs both :)16:08
katcohehe16:08
katcomgz: ta16:08
babbageclunkfwereade: still around?16:08
fwereadebabbageclunk, heyhey16:24
babbageclunkfwereade: hey, so now I'm trying to understand auth in the API.16:24
fwereadebabbageclunk, basically, your facade is passed a facade.Authorizer that tells you who made the (already-authenticated) connection to your facade16:26
fwereadebabbageclunk, often you can just return common.ErrPerm from the facade ctor when you know that the caller isn't allowed -- e.g. a user connecting to a facade intended for a worker16:27
babbageclunkfwereade: Ok, so there's the AuthModelManager() method, that is only true for something that's running on the controller? I'm using that in the ctor.16:27
babbageclunkfwereade: You mentioned in the review that AllMachineRemovals should take entities and check that it's the model tag.16:28
fwereadebabbageclunk, ah yes16:29
fwereadebabbageclunk, right, the AuthModelManager is the right one for the ctor16:29
babbageclunkDo I do that just by checking that the type of the tag is model, and then calling authorizer.AuthOwner(modelTag)?16:29
fwereadebabbageclunk, and... it is sort of ludicrous that the Authorizer *doesn't* have a ModelUUID representing when model it's connected to, but you*can* get that out of state16:30
fwereadebabbageclunk, AuthOwner isn't quite right16:30
babbageclunkfwereade: Maybe HasPermission?16:30
fwereade// AuthOwner returns true if tag == .GetAuthTag(). Doesn't need16:30
fwereade// to be on this interface, should be a utility fun if anything.16:30
fwereadeGetAuthTag will be the machine that connected16:31
fwereadebabbageclunk, I would (1) reject non-model-tags, and (2) reject model tags that don't match the state's modeluuid16:31
fwereadebabbageclunk, for bonus points, a ConnectedModel or something on Authorizer would be much nicer than having to ask state16:32
babbageclunkfwereade: Ah, right - not really looking at auth at all.16:32
fwereadebabbageclunk, yeah, but I think conceptually that info *should* be coming from the authorizer16:32
babbageclunkfwereade: yeah, that does sound nicer.16:32
fwereadebabbageclunk, it's just that it hasn't really evolved much at all over the years16:32
babbageclunkOk, I'll try to thread that through.16:32
fwereadebabbageclunk, <316:33
babbageclunkAnd should it accept multiple models? Presumably that would always be denied - the agent's connected to one model specifically, right?16:34
fwereadebabbageclunk, connections to a controller muddy the waters a little bit16:34
fwereadebabbageclunk, I would probably go no further than reporting *what* model the connection is for, and leave the interpretation of that in the facade, at least to begin with16:35
babbageclunkfwereade: Right, sorry. I meant that check in AllMachineRemovals facade method.16:36
fwereadebabbageclunk, ah, got you, yeah -- the connection is specifically for just one model16:37
fwereadebabbageclunk, so constructing a canAccess AuthFunc and passing that on to the rest of your logic is probably ideal16:37
babbageclunkfwereade: Cool. I'll do the same check on WatchMachineRemovals.16:37
fwereadebabbageclunk, that's a `func(names.Tag) bool` iirc16:38
fwereadebabbageclunk, perfect16:38
babbageclunkfwereade: And then the CompleteMachineRemovals and GetNetworkConfig ones already get lists of machines - so I do the checks against them?16:38
fwereadebabbageclunk, I think they're implicitly authorized by the AuthModelManager -- whereas if it were just some machine agent, we'd want to restrict info about any *other* machines16:39
fwereadebabbageclunk, but this is very much situational IYSWIM16:40
fwereadebabbageclunk, deciding what gets access to what is pretty much the facade's primary job16:40
fwereadebabbageclunk, we should generally be expecting to write the logic in terms of an AuthFunc chosen by the facade, or something very similar16:40
babbageclunkfwereade: Yeah, I think I see that.16:41
fwereadebabbageclunk, so that when we end up wanting to do the same stuff elsewhere, it's easy to pull that logic into a common/whatever type and parameterise it with an AuthFunc on facade construction16:41
fwereadebabbageclunk, e.g. apiserver/lifeflag, where I was very happy to just grab a couple of existing common services and specialise them16:42
babbageclunkfwereade: Oh, right - I see some things like that in the machiner.16:42
babbageclunkfwereade: Actually, something related that I was going to ask you about this morning but then got distracted by your review...16:43
fwereadebabbageclunk, yeah -- it's not all super-elegant but at least it separates the responsibilities, or lets us chalk out a path towards doing so16:43
fwereadebabbageclunk, oh yes?16:43
babbageclunkfwereade: ProvisionerAPI.GetContainerInterfaceInfo is very close to what I want for getting the network config for machines.16:44
fwereadebabbageclunk, then that's a great candidate for just that sort of extraction :)16:44
babbageclunkfwereade: Right, that was what I was thinking/wondering. It's slightly different because it only uses the first address on the device though.16:45
fwereadebabbageclunk, hmm... I'm not sure I have the context for a firm judgment there16:46
fwereadebabbageclunk, if there's no drawback to sending extra info to the one that doesn't need all of it, might still be sensible16:46
babbageclunkfwereade: ok, I'll think about that.16:47
fwereadebabbageclunk, there might be something genuinely in common, but there might not ;)16:47
fwereade(not always easy to tell)16:47
babbageclunkfwereade: might be better to just do my one and then see what's common, rather than the other way around.16:48
fwereadebabbageclunk, ack16:48
babbageclunkfwereade: ok, great - thanks! Sorry to bug you late!16:48
fwereadebabbageclunk, http://thecodelesscode.com/case/23316:48
fwereadebabbageclunk, no worries!16:48
babbageclunkfwereade: cute16:50
fwereadebabbageclunk, I like that site a lot, there's some good stuff there16:51
fwereadebabbageclunk, occasionally self-contradictory in the very best possible way16:51
mupBug #1611453 opened: manual-provider: systemd services left behind <ci> <kill-controller> <manual-provider> <regression> <systemd> <xenial> <juju-core:Triaged> <https://launchpad.net/bugs/1611453>16:52
natefinchkatco: sorry, was out.  Looks like you got it though.16:56
rick_h_perrito666: got a sec?16:59
perrito666rick_h_: certainly16:59
rick_h_perrito666: so pulled that pr down and built a juju with it and verified my local show.go has your patch in it17:00
rick_h_perrito666: but it's not working for me when I run $ djuju show-model controller17:00
rick_h_perrito666: I get the output for model "default"17:00
perrito666rick_h_: mm, something might have changed since I PRd that, tx for the test, ill re-review it and fix it and re-propose it17:01
rick_h_perrito666: rgr ok ty17:01
rick_h_perrito666: and let me know if I'm doing something wrong, only built things a couple of times so problem coould be on my end without a doubt17:01
perrito666rick_h_: well the way you describe it, the issue is most likely on my end :)17:02
rick_h_perrito666: k, let me know if I can be of any service17:02
=== frankban is now known as frankban|afk
=== ivyyy is now known as ivyyyy-brb
mupBug #1611463 opened: GUI: test_upgrade_gui failed KeyError: 'bootstrap-config' <ci> <juju-gui> <regression> <unit-tests> <juju-core:Triaged> <https://launchpad.net/bugs/1611463>17:58
katcosinzui: is there any easy way to check if lp:1596462 is fixed in master?19:03
sinzuikatco: Lets be optimistc and close it. http://reports.vapour.ws/releases/issue/5770f063749a56783c8c36fc shows we saw it once a week, but we haven't seen it in 4 weeks19:09
katcosinzui: well, i just landed something today that (in theory) should fix it19:09
katcosinzui: (last comment in the bug)19:09
balloonsrogpeppe, so I started making the changes to migrate everything from launchpad.net/gnuflag to github.com/juju/gnuflag. I fixed everything in juju/juju, but I see there's imports in several other repos. What's the best way to coordinate this change?19:10
sinzuikatco: I suspect this bug was caused by an overloaded server. We restructured tests to avoid cpu starvation19:11
katcosinzui: ah. well in any case, even under load the bug should be fixed19:11
sinzuiwhy was it only seen on Fridays?19:11
katcosinzui: not sure if that was a question for me. i don't know. weekend test run?19:15
sinzuikatco: jsut a public musing. CI doesn't schedule extra work on late Thursday early Friday. The pattern is odd19:17
mupBug #1609494 changed: grant-revoke: reenabled users missing from list-users <ci> <grant> <regression> <user> <juju-ci-tools:Invalid> <juju-core:Fix Released by axwalk> <https://launchpad.net/bugs/1609494>19:17
balloonsnatefinch, can I get you to review something for me / make sure I've done things properly?20:14
natefinchballoons: sure20:16
balloonsnatefinch, https://github.com/juju/juju/pull/5956. I'm attempting to land snapcraft.yaml and updating 2 depends off bzr to git instead. tomb and gnuflag20:16
natefinchballoons: wow, those were our last two launchpad dependencies, huh?20:20
* perrito666 cries in spanish20:21
natefinchballoons: for romulus, you updated tomb dependencies.tsv to use tomb.v1... but didn't update any dependencies that use tomb.  that'll probably break something in CI.20:23
balloonsnatefinch, yes indeed20:23
natefinch(I assume gnuflag doesn't use tomb)20:23
balloonsthey are the last. I was trying to find all the places it was used, but juju/* is quite big obviously. I also assumed perhaps I need to land the supporting PR's before, the main PR in juju/juju20:24
natefinchyes, you do need to land those first, and then update juju's dependencies to point to the new commits for those repos20:25
natefinchit's just a giant PITA20:25
balloonsnatefinch, so i'm not sure what you mean by the romulus comment20:27
perrito666brb20:27
natefinchballoons: you changed what dependencies.tsv says the repo depends on... but you didn't actually change any code that imports launchpad.net/tomb to import the github repo20:28
natefinchballoons: oh, I think I see what it is.  utils has the dependency on tomb, and romulus uses utils20:29
balloonsnatefinch, right. utils has the inverse problem20:30
natefinchactually... romulus also depends on juju/juju ... which is just a horrible idea20:30
balloonsi updated gnuflag, but it only has tomb underneath20:30
natefinchwait, gnuflag depends on tomb?20:31
natefinchoh nevermind20:31
balloonsnatefinch, no sorry.. I meant utils only uses tomb, not gnuflag, but depends on romulus that does use gnuflag20:31
natefinchI misunderstood20:31
balloonsI'm not sure why those sub-repos also have dependencies.tsv..20:31
natefinchthey really shouldn't.  The only thing that should have a dependencies.tsv is a repo that makes a binary20:32
perrito666natefinch: that is a strong statement20:34
balloonsnatefinch, ok, well do we feel I've gotten all the places that need updated?20:35
natefinchanyway, my point is.... you can't change a dependency path without changing either some codein the repo that uses it, or changing another dependency that had code changed to use it.20:35
natefinchthis change can't be correct: https://github.com/juju/utils/pull/230/files20:35
natefinchnothing is using the new gnuflag path20:35
natefinchwhich means something is likely using the old gnuflag path, which means this is wrong20:36
balloonsnatefinch, why is it in the dependencies.tsv to begin with then? nothing under utils uses it20:36
balloonsthat was I assumed the utils <-> romulus relationship.. I gues20:37
natefinchrelationships are never two way.  always only one way20:37
natefinchsomething utils imports is using gnuflag20:37
balloonsI can find no other uses anywhere20:38
balloonsdoes godeps actually take out dependencies that aren't needed?20:38
balloonsI felt like juju/juju has stuff it might not use for instance; not that I've explored it20:38
natefinchif you run godeps ./... it'll print out the actual dependencies20:38
natefinchor godeps -t ./... to include testing dependencies20:39
perrito666natefinch: what is wrong with the link you passed?20:39
natefinchand that includes gnuflag somehow..20:39
natefinchperrito666: gnuflag was updated in dependencies.tsv, but no imports that actually referenced the old one were updated to use the new one (and no other dependencies changed)20:40
perrito666ah I missed thatr20:40
natefinchahh, I see what it is20:41
natefinchjuju/cmd still references the old one.  You need to check in that change, then update utils to reference the new one20:41
balloonsthat makes sense to me20:42
natefinch(to reference the new juju/cmd)20:42
perrito666in github.com/juju there are 237 imports of gnuflags20:42
natefinchthis is why it's a PITA20:42
natefinchperrito666: almost all of those are in juju/juju20:42
perrito666sorry 15320:42
natefinchso it's a rolling update - juju/cmd is the leaf, check that in, update juju/utils to reference the new juju/cmd, check that in, update juju/romulus to reference the new juju/utils. check that in20:43
natefinchalthouhg romulus depends on juju/juju, so you'll need to do that before romulus20:43
perrito666yup, ironically utils only deppends on gnuflags but indeed has no dependency as you said, then there is terms-client, romulus and cmd20:43
natefinchand this is why only the top level should use dependencies.tsv.... because it's only the top level one that actually matters anyway.  It's not like juju runs the dependnecies.tsv in juju/utils20:44
perrito666natefinch: nah, that is bogus20:44
perrito666if you create a library, it makes perfect sense that the library has dependencies.tsv20:44
perrito666godeps is just an imperfect method, it should know how to handle cascades20:45
natefinchI guess you may need dependencies.tsv to build juju/utils to be even able to check stuff in20:45
perrito666natefinch: a library has the same issue than a binary20:45
perrito666we need vendoring :p20:45
natefinchvendoring is good, but it doesn't fix this problem.  utils would still need to vendor its own dependencies if master of the things it depends on won't build together20:46
natefinchthen you're just updating the vendor directories for each... not much better20:47
balloonsnatefinch, ok, I'll start with $$merge$$ on juju/cmd20:47
natefinchballoons: there, I gave you provisional LGTMs.  Just roll in the updates to dependencies.tsv as things get committed.20:52
natefinchballoons: and then feel free to land20:52
balloonson it20:52
natefinchballoons: why are you pushing these dependency updates, btw?  I mean, it's a good idea, but why you why now?20:54
balloonsnatefinch, ahh. I need it to make building a snap in launchpad possible20:54
balloonsnatefinch, there's a bug using bzr to pull depends inside the builder. This 'fixes' that bug20:54
natefinchballoons: I thought it might be that.  It's kind of nice, actually... the go tool complains when you tell it to download stuff from launchpad, since it's over http20:55
balloonsright.. this cleans up things for everyone, and gets rid of the bzr dependency as well20:56
balloonsso why not :-)20:56
* natefinch tries not to cheer too loudly.20:56
* katco quietly mentions that all our ci stuff is hosted in bzr20:57
* balloons notes it should not only be in git, but in the same repo(s) as juju itself20:58
* natefinch cries in the corner.20:58
katcoballoons: go get 'em :)20:58
perrito666well its also writen in python, so if you are going for consistency you have a long road ahead20:59
natefinchI was going to mention that...  sigh.20:59
katcoi don't know that it being written in python is a huge deal20:59
niedbalskidimitern, https://pastebin.canonical.com/162703/ latest run20:59
katcoand this is coming from someone who doesn't know python21:00
balloonsyea actually I think you should be glad they aren't written in go21:00
natefinchkatco: it's a barrier of entry for our go developers21:00
balloonsnatefinch, looks like loggo depends is out of date; I'm going to update it too. build failed21:00
natefinchballoons: cool21:00
natefinchI'm outta here for a while.  back in 3-4 hours.21:01
=== natefinch is now known as natefinch-afk
balloonsthanks natefinch-afk for the review21:01
katconatefinch: not a really high one imo. i'd sooner complain about the custom infrastructure built up for interacting with juju21:02
balloonsmmm.. indeed katco21:02
mupBug #1611514 opened: "local" charm schema should allow deploying previously deployed local charms <feature> <juju-core:New for cox-katherine-e> <https://launchpad.net/bugs/1611514>21:08
alexisbthumper, ping21:11
thumperyus?21:11
alexisbhttp://reviews.vapour.ws/r/5386/21:11
alexisbcan you please review this for perrito666 ^^21:11
thumperk21:11
thumperperrito666: conflicts against master21:17
thumperperrito666: also, bigger than 500 lines21:17
thumper:)21:17
perrito666aghh I am never again changing code that is called all over the place21:18
perrito666thumper: fixing the merge, could you make an exception with the lines ?21:21
* thumper is reviewing21:22
thumperssh21:22
perrito666I am prepared to pay in alcoholic beverages21:22
perrito666rick_h_: take a look at the branch,I fixed the issue21:29
rick_h_perrito666: ty, will have to wait until the morning21:29
rick_h_perrito666: but will do21:29
=== menn0 is now known as menn0-exercise
alexisbmenn0-exercise, liking dev snap, the setup script is mighty handy21:58
=== ses is now known as Guest93621
=== menn0-exercise is now known as menn0
menn0alexisb: yeah, it took a little while to figure out how to make that work (it's easy once you know how however)22:18
perrito666we should rename ship-it to amen so I can ask "can I get an amen" when asket for a shipit22:57
redirperrito666: halleluja!23:02
alexisbperrito666, did thumper give you a ship it?23:07
alexisbif so I will second redir's halleluja23:07
thumperalmost23:08
thumperhave now with one potential change23:09
alexisbhalleluja!23:09
perrito666alexisb: implementing change after standup and then its ok to go23:11
perrito666and we can use $$halleluja$$ for the merge23:11
alexisb:)23:11
rediraxw: got a minute?23:34
menn0alexisb, thumper: tech board agenda now has your items for today's meeting23:34
alexisbthank you menn023:34
axwredir: did you want to talk to me?23:34
rediryeah just a couple questions23:35
rediraxw: ^23:35
axwredir: yup, hangout or here?23:35
redireither but HO is prolly faster23:36
redirunless you remember where we put region config in globalSettingsC and have an idea where I would get the region and cloud from in modelConfigSources23:37
rediraxw: ^23:37
perrito666If the standup would have been its usual lenght i could have finished my recipe :p23:38
perrito666Standup + cookingshow23:39

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