| davecheney | or make StreamsVersion private | 00:00 |
|---|---|---|
| davecheney | either way | 00:00 |
| davecheney | once it's public people will starting using it for things | 00:00 |
| davecheney | think of the version.Version cluster fuck | 00:00 |
| === Ursinha is now known as Ursinha-afk | ||
| perrito666 | davecheney: I did | 00:06 |
| perrito666 | wait | 00:06 |
| perrito666 | davecheney: expand that | 00:06 |
| davecheney | perrito666: i cannot see any comments on reviewboard | 00:06 |
| davecheney | this may not be your fault | 00:07 |
| davecheney | it could be RB | 00:07 |
| perrito666 | davecheney: which one, I sent you two emails | 00:07 |
| davecheney | perrito666: did you place any comments on http://reviews.vapour.ws/r/18/ | 00:07 |
| perrito666 | no, I just LGTMd, I see you added a comment, I am checking what I missed now | 00:07 |
| perrito666 | I did add several comments to 33 though | 00:07 |
| davecheney | perrito666: cool | 00:08 |
| davecheney | to restate, this is not a critisism | 00:08 |
| davecheney | just checking that i'm not missing things recorded in reviewboard | 00:08 |
| davecheney | i find it's interface as comfortable as riding a tractor | 00:08 |
| perrito666 | davecheney: that is ok, I just went back to take a look just in case, I a a bit lost in RB still | 00:08 |
| davecheney | perrito666: 33 is NOT LGTM | 00:09 |
| perrito666 | davecheney: last tractor I rode was waaay better than my car | 00:09 |
| perrito666 | davecheney: odd, dimitern said that this change was suggested by you | 00:10 |
| davecheney | yup, he's fixed some todos | 00:10 |
| davecheney | which is awesome | 00:10 |
| davecheney | but changed the tags inside some structures which i'm super protective of | 00:11 |
| perrito666 | davecheney: <dimitern> re the tag changes - it was making writing tests unwieldy and dave already commented in a few places we need to fix this | 00:11 |
| perrito666 | he got carried away with changes | 00:11 |
| davecheney | perrito666: the reason we have to use a type assertion is the interface those types conforms to requries the Tag() method return a names.Tag, | 00:12 |
| davecheney | that can be fixed | 00:12 |
| davecheney | but the type stored inside the structure must be the concrete tag type that matches the thing it's tagged | 00:12 |
| davecheney | or in other wods | 00:12 |
| davecheney | type Machince struct { tag names.MachineTag } | 00:12 |
| davecheney | ^ anything else is going backwards | 00:12 |
| perrito666 | makes sense | 00:13 |
| perrito666 | davecheney: thanks for the explanation though | 00:20 |
| menn0 | thumper: another one: http://reviews.vapour.ws/r/25/ | 00:21 |
| perrito666 | davecheney: btw, do you see the comments I did there? | 00:22 |
| davecheney | perrito666: nope | 00:24 |
| davecheney | but i didn't read the whole thing | 00:24 |
| davecheney | just rage quit when i saw the change to the tags | 00:25 |
| perrito666 | mm, I seem to need to do some changes so others can see those | 00:25 |
| davecheney | gotta push the 'review' button | 00:25 |
| davecheney | which actually means 'post review' | 00:25 |
| davecheney | the | 00:25 |
| davecheney | the 'review' button is actually labeled 'view diff' | 00:26 |
| perrito666 | I did, odd | 00:26 |
| menn0 | thumper: with RB, if you're happy with a change you need to click "Ship it" or tick the option (depending on which screen you're on) | 00:26 |
| menn0 | thumper: I've just done it for #30 since you were happy with that one. | 00:26 |
| * perrito666 looks for a way to links to his comments and fails.. and burns the food in the stove | 00:26 | |
| thumper | kk | 00:27 |
| davecheney | thumper: i have to go out in 60 mins | 00:28 |
| davecheney | any chance of a chat before then ? | 00:29 |
| thumper | davecheney: sure, how about now? | 00:29 |
| davecheney | thumper: i'll see you in the standup channel | 00:29 |
| thumper | kk | 00:29 |
| waigani | megawatcher tests are passing | 00:46 |
| === Ursinha-afk is now known as Ursinha | ||
| jcw4 | wallyworld: is there any precedent for passing in partially filled results structs as filters/tempates for an API List* or Get* method in Juju? | 00:57 |
| wallyworld | jcw4: do you have a concrete example? | 00:58 |
| jcw4 | wallyworld: well for actions, I want to be able to List the actions queued up for a Service and/or a Unit, and possibly filter by Status (pending, running, finished, failed, etc.) | 00:59 |
| jcw4 | s%and/or%or% | 00:59 |
| wallyworld | you'd define a Filter struct to hold the query parameters? | 01:00 |
| jcw4 | wallyworld: I was thinking of using the return type struct and partially filling out the fields...? | 01:00 |
| wallyworld | yuk :-) | 01:00 |
| jcw4 | :) | 01:00 |
| wallyworld | IMHO | 01:00 |
| jcw4 | (that's why I asked) | 01:01 |
| wallyworld | :-) | 01:01 |
| wallyworld | np | 01:01 |
| jcw4 | I know mongo does something like that | 01:01 |
| jcw4 | I'm pulled between keeping type explosion down vs. simple API | 01:02 |
| wallyworld | it does? i didn't know that. I've just encountered Find(blah).One(&result) type stuff | 01:02 |
| wallyworld | sometimes simple != intuitive | 01:02 |
| jcw4 | wallyworld: I interpreted the blah as a 'template' | 01:02 |
| wallyworld | eg | 01:03 |
| wallyworld | s.db.C("managedStoredResources").Find(bson.D{{"path", "environs/env" + path}}).One(&mrDoc) | 01:03 |
| wallyworld | blah = a list of filter conditions | 01:03 |
| jcw4 | wallyworld: okay.. so instead of one Get method, maybe GetActionsOn(service names.Tag), GetFailedActionsOn(service names.Tag), etc... | 01:04 |
| jcw4 | or GetActionsByStatus(entity names.Tag, status string) etc. | 01:04 |
| wallyworld | the latter would be preferable | 01:05 |
| jcw4 | and, modified to be all bulk calls of course | 01:05 |
| wallyworld | i would imagine under the covers it would call GetActions(filter) | 01:05 |
| jcw4 | wallyworld: yes. | 01:05 |
| jcw4 | okay, so slightly more verbose API would be preferable | 01:06 |
| wallyworld | that works for me personally. i like a wide API at the level of the caller, and a narrow api over the wire | 01:06 |
| jcw4 | wallyworld: hence the GetActions(filter) under the covers I suppose? | 01:07 |
| wallyworld | yep | 01:08 |
| wallyworld | a narrow api over the wire is more robust against change | 01:09 |
| wallyworld | eg if a new status were added | 01:09 |
| jcw4 | makes sense | 01:09 |
| wallyworld | axw: hi there, i booted an aws environment from trunk using upload-tools, and when I try and add-machine, I get | 01:50 |
| wallyworld | "1": | 01:50 |
| wallyworld | agent-state-info: no matching tools available | 01:50 |
| axw | :/ | 01:50 |
| wallyworld | have you seen that? | 01:50 |
| axw | nope | 01:51 |
| wallyworld | ok, i'll poke around | 01:51 |
| rick_h_ | wallyworld: what's this placement/ensure availabiity stuff? | 02:06 |
| rick_h_ | wallyworld: you guys going to give me a manual api back? | 02:06 |
| wallyworld | rick_h_: the landscape guys want to use ensure-availability and be able to nominate what machines should become the new HA servers | 02:06 |
| wallyworld | we already have this for bootstrap | 02:06 |
| rick_h_ | wallyworld: what about UI guys that want to have ensure-availability by default, but allow people to override if they want to? | 02:07 |
| wallyworld | it's extending that to ensure-availability | 02:07 |
| rick_h_ | wallyworld: ah ok, so not what I'm hoping for | 02:07 |
| wallyworld | what do you mean ensure-availability by default? | 02:07 |
| rick_h_ | so that is on by default on azure | 02:08 |
| rick_h_ | which is cool, I like safe defaults and all that | 02:08 |
| rick_h_ | but that means by default, no user can specify placement | 02:08 |
| rick_h_ | they have to rebootstrap without the ensure-availability and then don't get any help | 02:08 |
| wallyworld | yes, that is an azure limitation :-( | 02:09 |
| rick_h_ | right | 02:09 |
| wallyworld | this change is primarily for maas | 02:09 |
| rick_h_ | but my understanding is that it's only a limitation because we made it that way | 02:09 |
| wallyworld | i don't think so, axw would know more | 02:09 |
| axw | no, it was made that way to work around azure's restrictive nature | 02:10 |
| rick_h_ | there's no way to setup 'I don't care if these are HA' services manually placed, log a log aggregator or *cough* jenkins *cough* install | 02:10 |
| axw | rick_h_: the way azure does HA is very different to other cloud providers, which means we can't support both at the same time | 02:11 |
| rick_h_ | axw: right, but I'm suggestions, or I guess asking, if there's room for some things to not be HA in an env | 02:11 |
| rick_h_ | /suggestions/suggestion | 02:11 |
| axw | rick_h_: yes, with some work we could probably do it on a per-service basis | 02:12 |
| axw | I don't think it's a small amount of work though | 02:13 |
| rick_h_ | axw: yea, in brussels I'll probably ask about it. Especially with the planned work for more auto placement, it kind of breaks the whole chunk of work we've done in machine view | 02:13 |
| rick_h_ | axw: on the one hand we're looking at trying to enable colocating and collapsing down, but if we start auto placing and turning off overrides, then we break that story for smaller/getting started users | 02:13 |
| rick_h_ | and even stuff like the orange box setup where almost every node there has multiple services on it | 02:14 |
| wwitzel3 | axw: no need to worry about the versioning stuff for juju run .. I spoke to william and he completely extended the scope of my work and added some rather large, non-trivial things to that PR ;) | 02:14 |
| axw | wwitzel3: lol | 02:14 |
| wwitzel3 | axw: good news is, it will be versioned :) | 02:14 |
| rick_h_ | axw: wallyworld ok, saw a branch go by with an interesting little title and had to ask what was up. | 02:14 |
| axw | rick_h_: I understand, we do need to do something about it | 02:15 |
| wallyworld | rick_h_: np at all, ask anytime | 02:15 |
| axw | it's a bit crappy having to choose | 02:15 |
| axw | at such a coarse level anyway | 02:15 |
| wwitzel3 | axw: I'm just making a new, smaller, focused facade for the run part of the client, instead of leaving it all rolled up under client. | 02:15 |
| axw | wwitzel3: cool, SGTM, just like what wallyworld did for ensure-availability | 02:15 |
| rick_h_ | axw: yea, my feeling on it. I think it's good stuff, just a bit :( since it's by default and the answer is "once you realize you DO want to colocate something, rebootstrap" | 02:16 |
| wallyworld | and eric for Backup, and also previously for KeyManager, UserManager etc | 02:16 |
| axw | wallyworld: I hate private image metadata | 02:17 |
| axw | :( | 02:17 |
| wallyworld | sadly we need it :0( | 02:17 |
| wallyworld | :-( | 02:17 |
| wwitzel3 | axw: yep, and the usermanager facade | 02:17 |
| axw | it's messing up my nice abstractions | 02:17 |
| wallyworld | wwitzel3: and keymanager and backup | 02:17 |
| wallyworld | axw: did what we discuss last night about adding it in to the current CustomMetadata work flow turn out? | 02:18 |
| axw | wallyworld: I've got an API client-based one jammed in there. feels a bit wrong to be calling the API from there though... | 02:19 |
| axw | also, going to need to do something else to read the image metadata at bootstrap time | 02:19 |
| axw | previously we'd upload to cloud storage before bootstrapping, don't have that luxury now | 02:19 |
| wallyworld | axw: we should use a bespoke interface and inject in an implementation | 02:20 |
| wallyworld | for retrieving image metadata | 02:20 |
| wallyworld | the interface lives in provider | 02:20 |
| wallyworld | there's then an api cient adaptor | 02:20 |
| wallyworld | or something like that | 02:21 |
| axw | wallyworld: it's too error prone... we'll end up missing a call to set the source | 02:21 |
| wallyworld | that's what tests are for :-) | 02:21 |
| axw | tests where? *everything* uses an Environ, and we add it all the time | 02:21 |
| wallyworld | and we can have the thing which constructs the provider even read stuff from state, and pass in static data behind that interface | 02:21 |
| axw | environs.New(*config.Config) | 02:22 |
| wallyworld | hmmm | 02:22 |
| wallyworld | maybe it needs to be done on the provider | 02:23 |
| wallyworld | cause that's where startinstance is | 02:23 |
| axw | no it's not, that's on Environ | 02:23 |
| wallyworld | ah, yeah | 02:23 |
| axw | EnvironProvider just opens/prepares | 02:23 |
| axw | more or less | 02:24 |
| wallyworld | it's at that point we could pass in the metadata | 02:24 |
| wallyworld | this is all handwavy without looking back at the code | 02:25 |
| waigani | i'm getting the following error: /usr/lib/go/pkg/tool/linux_amd64/6l: conflicting definitions for @"github.com/juju/juju/state".State | 02:30 |
| waigani | is it safe to delete /usr/lib/go/pkg/ ? | 02:32 |
| waigani | I deleted ~/go/pkg but it didn't help | 02:32 |
| waigani | davecheney: ^? | 02:32 |
| axw | waigani: I don't think you'd want to remove /usr/lib/go/pkg, unless you installed a package there with sudo | 02:33 |
| waigani | axw: no I didn't | 02:33 |
| waigani | axw: I'm using the go that comes with 14.04 | 02:34 |
| axw | wallyworld: I'm going to investigate another approach of having a datasource registry in environs/imagemetadata | 02:34 |
| wallyworld | ok | 02:34 |
| axw | waigani: ok, dunno what that error is, but removing that dir sounds wrong | 02:35 |
| waigani | yeah, felt wrong, that's why I asked. thanks | 02:35 |
| waigani | actually its referring to a bunch of stuff in tmp | 02:35 |
| waigani | that I can delete | 02:35 |
| waigani | e.g /tmp/go-build964096169/github.com/juju/juju/state/_test/github.com/juju/juju/state_test.a(_go_.6): | 02:35 |
| axw | wallyworld: the registry will be keyed on environ. may make it possible to remove the SupportsCustomSources interfaces | 02:36 |
| wallyworld | worth a spike | 02:37 |
| waigani | except that its already been deleted... hmm | 02:37 |
| waigani | fixed it, that was a new one | 02:41 |
| menn0 | wallyworld: do you need a review for this? http://reviews.vapour.ws/r/38 | 02:43 |
| wallyworld | menn0: yes please | 02:43 |
| menn0 | wallyworld: doing it now | 02:43 |
| wallyworld | ty | 02:44 |
| wallyworld | menn0: i pushed a small fix to the github pull request, what's the command to update the reviewboard diff? just "rbt post" ? | 02:56 |
| menn0 | wallyworld: "rbt post -u" might do it... or else "rbt post -r <review number>" | 02:57 |
| wallyworld | ta | 02:57 |
| wallyworld | axw: found the tools problem. i had an older compiled jujud in my path (1.20.8) and it found and uploaded that one and then when that 1.20.8 jujud went to find tools, it looked for 1.20.8 and there were none as juju recorded tools for 1.21 | 03:16 |
| axw | wallyworld: ah, ok. | 03:17 |
| axw | that's a problem with upload-tools, it just assumes whatever it finds is the same as the CLI version | 03:18 |
| wallyworld | yeah, so i found out :-( | 03:18 |
| wallyworld | i didn't even realise i had a binary, and i didn't notice the log message n amongst all the noise | 03:18 |
| menn0 | wallyworld: just finished reviewing http://reviews.vapour.ws/r/38/. thumper can you do a meta-review please? | 03:26 |
| wallyworld | thanks menn0 | 03:26 |
| * thumper looks | 03:27 | |
| thumper | menn0: I can't see your comments on the main diff | 03:31 |
| thumper | ahh... | 03:32 |
| thumper | because the comment is on diff one | 03:32 |
| menn0 | thumper: ah... wallyworld uploaded a new diff while I was reviewing... | 03:32 |
| menn0 | thumper: if you're looking at my review comments, you can click the title (where the filename is) to jump to the code. | 03:33 |
| wallyworld | menn0: thumper: i am going to back out that change i think - the issue i had from from using an old jujud. i thought i messed up the api versioning as i just guessed based on reading the code, but it turns out i was right after all | 03:33 |
| wallyworld | but i will keep the extra test | 03:34 |
| wallyworld | menn0: thumper: although, I do like the fact that the new implementation gets the version directly off the ha client. but it means possibly opening a ha client and closing it again if the version is wrong. what implementation do you guys prefer? | 03:37 |
| wallyworld | the first one gets the facade version by asking for it by name | 03:38 |
| wallyworld | the second one gets a client and asks the client | 03:38 |
| thumper | menn0, wallyworld: why has noone updated the branch in the .reviewboardrc? | 03:39 |
| wallyworld | nfi | 03:40 |
| menn0 | thumper: I think we needed to establish whether everyone has the "upstream" remote set up in the same way | 03:41 |
| thumper | menn0: if they followed the instructions they should have | 03:42 |
| thumper | menn0: if not, they need to fix it | 03:42 |
| thumper | menn0: why make the rest of us suffer? | 03:42 |
| menn0 | thumper: yeah.. and no-one said anything when I mentioned it on the list | 03:42 |
| menn0 | thumper: let's fix it | 03:42 |
| menn0 | thumper: I've been running rbt post with --tracking-branch=upstream/master in the mean time | 03:43 |
| * thumper nods | 03:43 | |
| * thumper submits his first review board review | 03:43 | |
| thumper | http://reviews.vapour.ws/r/39/diff/ | 03:45 |
| menn0 | thumper: will take a look soon | 03:45 |
| thumper | ta | 03:46 |
| wallyworld | jam: hi, i'd like your input on determining API version. http://reviews.vapour.ws/r/38/ The first diff did this: root.BestFacadeVersion("HighAvailability"), the second diff does this after instantiating a client: client.BestAPIVersion(). Both work. The first way doesn't require creating a client only to potentially not use it if the API doesn;t exist, but does require knowing the facade by name. What would you prefer? | 04:16 |
| wallyworld | this is in cmd/juju/ensureavailability.go | 04:17 |
| jam | wallyworld: so, I haven't looked at the whole diff, but is the idea that you're moving EA into another location? | 04:17 |
| jam | so IMO, you would have an api/ha/client.go that would try to use the new location if it existed, else fall back to the old location | 04:18 |
| jam | in which case, you'd still be wanting the client | 04:18 |
| jam | it would just be using the right API call underneath | 04:18 |
| wallyworld | jam: the facade name is different | 04:19 |
| wallyworld | i'm moving methods off the original big client | 04:19 |
| jam | wallyworld: that doesn't mean you need a different client side type | 04:19 |
| wallyworld | and onto a whole new facade | 04:19 |
| jam | wallyworld: sure, but the api/highavailability/client.go can still call APICall("Client", 0, "", "EnsureAvailability", ...) | 04:20 |
| wallyworld | oh,i see | 04:21 |
| wallyworld | that sounds nicer | 04:21 |
| wallyworld | jam: i'll implement that and get you to +1 if that's ok | 04:21 |
| jam | wallyworld: so it depends *what* we can get away with, but my idea is that we should push the compatibility layer as low as we can | 04:21 |
| wallyworld | yep | 04:21 |
| wallyworld | agreed | 04:21 |
| wallyworld | i didn't know how to do that | 04:22 |
| jam | wallyworld: so if we can hide compat underneath api/foo.go then do so | 04:22 |
| wallyworld | +1 | 04:22 |
| jam | sometimes it has to be done in client code like cmd/juju level code | 04:22 |
| jam | but for this, I think you can do it lower. | 04:22 |
| wallyworld | that would be good | 04:22 |
| jam | wallyworld: fwiw, I would rather it not be called apiserver/highavailability/client.go | 04:23 |
| jam | "client" here seems confusing. | 04:23 |
| jam | wallyworld: api/highavailability/highavailability.go seems slightly better (if a bit longer) | 04:23 |
| wallyworld | fair point, i cargo culted that from somewhere | 04:23 |
| wallyworld | ill fix | 04:23 |
| jam | wallyworld: apiserver/client/client.go I'd imagine | 04:24 |
| wallyworld | could be | 04:24 |
| wallyworld | jam: i've pushed the changes to rb if you would be kind enough to look | 04:40 |
| menn0 | thumper: wow, such diffs, very long | 04:40 |
| menn0 | thumper: almost done | 04:40 |
| thumper | menn0: ack, and ta | 04:41 |
| menn0 | thumper: done | 04:46 |
| davechen1y | thumper: thanks for the chat earlier | 04:50 |
| davechen1y | i've had a good mull over it | 04:50 |
| thumper | davechen1y: np | 04:50 |
| davechen1y | i think the multiwatcher should only accept types defined in its package | 04:51 |
| davechen1y | i won't say it is the only sane way to do it | 04:51 |
| davechen1y | but it feels more sane than what we have at the moment | 04:51 |
| davechen1y | holy shit | 04:59 |
| davechen1y | thumper: | 04:59 |
| davechen1y | type EntityId struct { Kind string Id interface{} | 04:59 |
| davechen1y | } | 04:59 |
| davechen1y | look, it's a tag!! | 04:59 |
| thumper | davechen1y: yeah, I've been looking at that information now in the watcher... | 05:00 |
| davechen1y | thumper: i keep getting confused between state/megawatcher, and state/multiwatcher | 05:00 |
| thumper | me too | 05:01 |
| davechen1y | thumper: i'll be spending 1/2 of tomorrow and all of friday on arm stuff | 05:04 |
| thumper | ack | 05:04 |
| davechen1y | now we have a machine I suspect aram's output will increase quite a lot | 05:04 |
| urulama | wallyworld: morning ... is there anything that you'd like to discuss today on a regular catchup? | 05:18 |
| wallyworld | urulama: hey, morning. | 05:18 |
| davechen1y | thumper: i'm thinking that params.EntityId should be the state versino of a tag | 05:22 |
| davechen1y | william has said many times he doesn't want tags stored in the db | 05:22 |
| davechen1y | but it feels like there is a case of a tag like think | 05:22 |
| davechen1y | and entity id may be that thing | 05:22 |
| davechen1y | it's an identified and a type bound together | 05:22 |
| thumper | hmm... interesting | 05:22 |
| davechen1y | honestly entitydi could be | 05:22 |
| davechen1y | type MachineEntity interface{} | 05:23 |
| davechen1y | well maybe, that might be a bad idea | 05:23 |
| davechen1y | but that sort of thing | 05:23 |
| davechen1y | it's a typed thing | 05:23 |
| davechen1y | and that is _exactly_ what an interface is | 05:23 |
| * thumper nods | 05:23 | |
| thumper | sounds reasonable... | 05:23 |
| davechen1y | i haven't quited figured out what entity id's do yet | 05:24 |
| davechen1y | they seem to be a way of informing watchers what _type_ of entity changed | 05:24 |
| thumper | I was just looking at the entity stuff as a wip review of jesse's look at adding the env-uuid to the service collection | 05:24 |
| thumper | I'm wanting the id to be environment specific | 05:24 |
| thumper | so the actual document ID is an opaque transformation of the entity id | 05:24 |
| davechen1y | oh yeah, they completely fail to capture that | 05:24 |
| davechen1y | they are at best a tupple { collection, row } | 05:24 |
| * thumper nods | 05:25 | |
| thumper | it could be looked at like that | 05:25 |
| davechen1y | so, the question is, do we extend the tupple to be | 05:25 |
| thumper | but I think it would be good to work out the real intent | 05:25 |
| davechen1y | { collection, env-uuid, row } | 05:25 |
| thumper | to work out how best to model this | 05:25 |
| thumper | I don't think that bit is necessary, as the state instance has an implicit env-uuid | 05:25 |
| davechen1y | or stick with { collection, row} and accept that GetAll does not return the complete set of documents in the collection | 05:25 |
| thumper | which is why I think the mega watcher needs to be environment specific | 05:25 |
| thumper | the later of what you said | 05:26 |
| thumper | I think | 05:26 |
| davechen1y | i need to figure out where that data is begin used | 05:26 |
| davechen1y | i'm guessing it's somethinglike | 05:26 |
| thumper | yeah, I agree | 05:26 |
| davechen1y | switch collection { | 05:26 |
| davechen1y | case "machines": state.GetMachine(id) | 05:26 |
| davechen1y | case "unit"L: state.GetUnit(id) | 05:27 |
| davechen1y | and so forth | 05:27 |
| davechen1y | which is so like tags it's not funny | 05:27 |
| davechen1y | thumper: why do all the backingNNN methods in megawatcher.go return an error | 05:32 |
| davechen1y | yet they are all hard coded to return nil | 05:32 |
| davechen1y | ? | 05:32 |
| thumper | NFI | 05:32 |
| davechen1y | store.Update(info) | 05:32 |
| davechen1y | return nil | 05:32 |
| davechen1y | } | 05:32 |
| * davechen1y feels a branch coming on | 05:32 | |
| thumper | because someone thought it was a good idea? | 05:32 |
| thumper | davechen1y: it would almost be "state.FindEntity(tag)" for the switch above | 05:33 |
| davechen1y | i wonder if they have to fit some interface | 05:33 |
| davechen1y | ./megawatcher.go:458: cannot use (*backingRelation)(nil) (type *backingRelation) as type backingEntityDoc in assignment: | 05:34 |
| davechen1y | hmmm | 05:34 |
| thumper | menn0: so when I update this review, do I squish locally first? | 05:48 |
| thumper | what is the normal practice? | 05:48 |
| menn0 | thumper: you can do whatever you like. review board will just upload the diff of your branch to upstream/master, regardless of how many commits it's in | 05:49 |
| thumper | ok | 05:49 |
| thumper | menn0: is there a keyboard shortcut to take me to the next review comment? | 05:50 |
| thumper | that'd be handy | 05:50 |
| menn0 | I haven't looked but I'd like to know too if there is one | 05:51 |
| * thumper looks for the rbt command to update | 05:52 | |
| * thumper is off now. | 05:54 | |
| menn0 | wallyworld: thumper is EOD so could you do a meta-review for this please? http://reviews.vapour.ws/r/34/ | 06:07 |
| wallyworld | sure | 06:07 |
| menn0 | wallyworld: otherwise ericsnow will have to wait longer to get his review | 06:07 |
| wallyworld | and we don't want that :-) | 06:07 |
| wallyworld | in case we need to ask for a reviewboard fix :-) | 06:08 |
| menn0 | wallyworld: :) | 06:21 |
| menn0 | wallyworld: cheers | 06:21 |
| wallyworld | menn0: 3/4 of the way through | 06:21 |
| jam | wallyworld: so your patch doesn't actually get the backwards compatibility stuff quite right. | 06:27 |
| wallyworld | ok | 06:27 |
| jam | wallyworld: you mention "regression tested on AWS", does that mean you did "juju-1.20 bootstrap" and "juju-1.21 ensure-availabilty" ? | 06:28 |
| wallyworld | jam: yes | 06:29 |
| wallyworld | but if i got the backwards compat stuff wrong, i will retest | 06:29 |
| jam | wallyworld: so you don't actually fall back to calling the Client API | 06:30 |
| jam | AFAICT | 06:30 |
| jam | HA didn't exist in 1.20, right? | 06:30 |
| wallyworld | jam: i think you are right, i tested before i did the last lot of changes after talking to you | 06:31 |
| jam | wallyworld: AFAICT, your legacy test is only patching the client side, so if it makes the new API call, the server still would accept it. | 06:31 |
| wallyworld | yeah, i'll have to rework it i think and retest | 06:32 |
| jam | so you really want a server that doesn't think it has the new functions, so that you know the client isn't requesting them | 06:32 |
| jam | TheMue should have some examples he's putting together | 06:32 |
| wallyworld | or i could patch out the client and check the facade called | 06:33 |
| jam | wallyworld: I feel like that still gets you into a place where you have to be careful to patch everything that might be touched | 06:33 |
| jam | you could | 06:33 |
| wallyworld | sure, yeah, i'll run up one end-end test | 06:33 |
| wallyworld | i'll look for the doc | 06:34 |
| wallyworld | thanks | 06:34 |
| wallyworld | jam: for some reason, reviewboard doesn't show the client.go files as renamed; the github pr does | 06:35 |
| dimitern | morning | 07:09 |
| dimitern | jam, hey, would you like to have a look at my http://reviews.vapour.ws/r/33/ please? | 07:23 |
| jam | dimitern: looking | 07:33 |
| dimitern | jam, thanks | 07:33 |
| dimitern | jam, in particular I'm having some trouble understanding why "s.firewaller.Machine(s.machines[0].Tag())" is worse than "s.firewaller.Machine(s.machines[0].Tag().(names.MachineTag))" | 07:37 |
| jam | dimitern: I'm not personally very sure why MachineTag is better/worse than just Tag, for that you'd have to have a discussion with davecheney | 07:38 |
| jam | (I certainly thought the point of Tag was that it was the abstract representation, so having type-specific versions of it seems odd) | 07:39 |
| jam | dimitern: oh, and *IMO* ever doing a plain type assertion: .(names.MachineTag) in production code is asking for a panic to bring your system down | 07:40 |
| jam | panic() ==> the whole thing dies | 07:40 |
| dimitern | jam, strictly speaking, using names.Tag internally is less safe than names.MachineTag, but the apiserver facade validates all tags it gets, so there's no way to pass a unit tag to a method allowing only machine tags and not get an error | 07:40 |
| jam | error() => we return "sorry can't do that" | 07:40 |
| dimitern | jam, ok so why state.Machine.Tag() returns names.Tag rather than names.MachineTag ? | 07:41 |
| dimitern | jam, I think in the API we should follow the same logic | 07:41 |
| jam | dimitern: *If* we are supposed to be using names.MachineTag, then it seems like it should | 07:41 |
| jam | dimitern: as stated, *I* don't really understand why | 07:42 |
| dimitern | jam, oh, ok - so we're in the same boat on this :) | 07:42 |
| jam | dimitern: my point was just that a bare type assertion seems to mostly be the wrong thing to do | 07:42 |
| dimitern | where are thumper and davecheney when you need them :) | 07:43 |
| jam | dimitern: certainly IMO MachineTag should be an implementation detail of Tag | 07:43 |
| jam | dimitern: also note that NewUnitTag was something that exists, and dave has stated "it is there for tests" but we don't have another way to go from "unit/0" to a UnitTag | 07:44 |
| dimitern | jam, I agree unchecked type assertions are bad, but if state.Machine.Tag() returns anything else than a names.MachineTag wrapped into a names.Tag interface, I'd rather panic than let it slip :) | 07:44 |
| jam | (it also used to panic if you gave it bad data, which meant that charms could crash the API server) | 07:44 |
| jam | dimitern: panic if developers are writing bad assumptions, but don't panic if you were given bad data | 07:45 |
| dimitern | jam, I think I understand their point - the internal tag filed of the Machine struct should be names.MachineTag, but firewaller.Machine() should take names.Tag and possibly ParseMachineTag and verify internally | 07:46 |
| dimitern | s/filed/field/ | 07:46 |
| jam | dimitern: so NewFacadeCallerForVersion seems like a bit of a "we don't really want to do it this way", but I suppose the point is to allow the server to go to a new version before the client is ready to handle it? But we can just do that by not incrementing the best-known version in api/facadeversions.go | 07:46 |
| jam | though I guess the test suite will tell you off for that... | 07:46 |
| dimitern | jam, no, it's not about the client | 07:47 |
| dimitern | jam, it's a way to implement a new server and client version of the api, test it and only switch over to it once it's user (i.e. the worker) is ready | 07:47 |
| dimitern | jam, I thought a lot about how to do it nicely, and I'm keen on your thoughts about my approach (both server-side and client) | 07:49 |
| jam | dimitern: so this is actually because we can't abstract the API difference, and we need 2 different workers, thus we are forced to explicitly chose which version the worker is talking to | 07:50 |
| jam | ? | 07:50 |
| dimitern | jam, in this case, having old and new versions of the worker using v0 and v1 is the easiest way, until we can drop the old worker and rely on having apiserver supporting v1 | 07:51 |
| dimitern | jam, in reality, once we have apiv1 in place and the upgrade step done, I think it will probably be safe to drop the v0-only worker, isn't it? | 07:52 |
| dimitern | jam, the only problematic case is when a MA running v1 firewaller worker is talking to a v0 apiserver, but since upgrades are done in lock-step, can we have this case at all? | 07:54 |
| jam | dimitern: you mean, we are unlikely to be talking to a v0 API server if the Agent is V1 ? | 07:54 |
| jam | I do believe that is true | 07:54 |
| jam | dimitern: well, if they "finished" the upgrade path wrt HA upgrades | 07:54 |
| jam | I'm not sure if menno has landed all of the pieces there | 07:54 |
| dimitern | jam, yeah, so the old v0 worker will be kept only until upgrade is done | 07:54 |
| mattyw | morning all | 07:55 |
| dimitern | jam, if not (i.e. we happen to connect to a v0-only apiserver in HA configuration, which haven't yet updated), I think it will be also safe to terminate the worker and restart it, hoping it will connect to an upgraded api server | 07:56 |
| dimitern | morning mattyw | 07:56 |
| jam | morning mattyw | 07:56 |
| dimitern | wallyworld, are you around? | 07:57 |
| wallyworld | hey | 07:57 |
| dimitern | wallyworld, hey, can you confirm the behavior in HA setup wrt server upgrades ^^ | 07:57 |
| wallyworld | maybe, i'll read | 07:58 |
| dimitern | wallyworld, I mean, suppose we have a new version that has an upgrade step to migrate stuff in the db, and then a worker starts after the upgrade expecting to find apiserver supporting the new api v1 it needs | 07:58 |
| dimitern | wallyworld, in HA-scenario there can be (I guess) some apiserver not yet upgraded - or we ensure all of them are and we then start any other workers? | 07:59 |
| wallyworld | dimitern: i *think* they are all upgraded, but am not sure. in any case, the worker will exit and restart i would expect | 08:00 |
| wallyworld | dimitern: i think there was some code landed recently about coordinating upgrades | 08:01 |
| dimitern | wallyworld, yeah, and keep doing this until it connects to an api server supporting the facade version the worker needs | 08:01 |
| wallyworld | yes, but if i am correct, upgrades should be coordinated as of recently | 08:01 |
| wallyworld | so either way, i think we are ok | 08:01 |
| jam | dimitern: so for agents we can go with a bit of "it doesn't have to work until the API server is new enough", I don't want us to get *too* used to that kind of programming. | 08:01 |
| jam | dimitern: iow, I'd rather we just make it work with v0 and v1 as much as we can, but this is a case where the difference is large enough it probably isn't worthi t. | 08:02 |
| jam | worth it | 08:02 |
| wallyworld | jam: i would think that some sites may want to run with an older api server for bit? upgrade in stages? | 08:02 |
| wallyworld | not sure if we plan to suport that | 08:02 |
| dimitern | wallyworld, my original question was "given 2 versions of a worker - old one using apiv0 and new one using only apiv1; when is it safe to remove the old v0 worker code - is it once we have an upgrade step in place that does migration apiv1 expects?" | 08:03 |
| wallyworld | hmmmm, good question | 08:03 |
| jam | wallyworld: I can see from a "I'm managing my infrastructure" wanting to do rolling upgrades, and testing various pieces. It hasn't been part of how "juju upgrade-juju" works at all. | 08:03 |
| wallyworld | jam: i think it will/could very well come up as a requirement, given people already want rolling charm upgrades perhaps | 08:04 |
| jam | wallyworld: so my personal opinion is that as much as possible clients and servers should support the widest range of versions that we can cope with as programmers. | 08:05 |
| wallyworld | yeah, i tend to agree | 08:05 |
| jam | And we're being pickier about that for Client apis vs Agent apis | 08:05 |
| wallyworld | we have little choice for clients | 08:05 |
| jam | well, there are people that would have said that if we had gone with an API from the beginning, "juju upgrade-juju" wouldn't exist, and we wouldn't need simplstreams, because the Agents would just be a "apt-get install jujud" | 08:06 |
| dimitern | for clients, absolutely, but we can be laxer with agents | 08:06 |
| jam | and we'd have to cope with Precise being a different version than Trusty | 08:07 |
| dimitern | jam, wallyworld, so I propose to trust our upgrade code does the right thing (and file bugs when not), and keep the old code only until the upgrade step is in place | 08:08 |
| jam | dimitern: I'd sign off on that, you may want to check with fwereade at some point to help it get codified as appropriate practice. | 08:09 |
| wallyworld | +1 to trusting the upgrade code | 08:09 |
| voidspace | morning all | 08:09 |
| wallyworld | o/ | 08:09 |
| dimitern | ok, I'll send a mail to juju-dev | 08:09 |
| dimitern | morning voidspace | 08:09 |
| wallyworld | william is moving today i think | 08:10 |
| voidspace | dimitern: o/ | 08:10 |
| jam | morning voidspace | 08:10 |
| voidspace | o/ | 08:11 |
| jam | TheMue: can you take a look at http://reviews.vapour.ws/r/33/diff/# Dimiter has an interesting approach to doing the versioned testing, which I don't think is quite what we had been discussing. | 08:11 |
| jam | (a base test suite that has a lot of helper functions that take an appropriate interface in each method) | 08:11 |
| jam | and then concrete tests that just thunk over to the helpers | 08:12 |
| jam | dimitern: as far as that goes, I feel like we're still feeling out how we want to do compatibility testing, etc. | 08:12 |
| dimitern | wallyworld, moving where? | 08:12 |
| wallyworld | flight back from UK I think he said | 08:12 |
| dimitern | ah, I see | 08:13 |
| wallyworld | he was away for a bit last week | 08:13 |
| jam | dimitern: I think William was advocating that we could do per API suites, and then aggregate them into versions of facades | 08:13 |
| jam | so you would have a WatchSuite, and an InstanceId suite, etc. | 08:13 |
| TheMue | morning | 08:13 |
| jam | morning TheMue | 08:13 |
| TheMue | jam: yep, will take a look | 08:13 |
| dimitern | jam, that sounds like a horrible amount of boilerplate code - imagine doing this for the uniter api | 08:14 |
| dimitern | morning TheMue | 08:14 |
| jam | dimitern: well, you have a fair amount of boilerplate in your testing of Firewaller | 08:14 |
| TheMue | jam, dimitern: seen my document where I tried to describe the testing approach? | 08:15 |
| jam | TheMue: https://github.com/juju/juju/pull/758/files ? | 08:16 |
| TheMue | jam: yes, and thankfully by clicking on "View" the markdown is rendered for better readability | 08:16 |
| dimitern | jam, I can't decide yet whether having a test-suite-per-api-method is more or less boilerplate than having "kinda C++-template-like helpers" taking an interface with a single method that's being tested | 08:18 |
| dimitern | TheMue, no, I'll have a look, but look at my proposal when you have some time (I'm quite proud what I came up with :) | 08:19 |
| * jam needs food, bbiab | 08:19 | |
| dimitern | TheMue, the approach taking a callback func (st *state.State, resources *Resources, authorizer Authorizer) (interface{}, error) is something I tried as well | 08:20 |
| TheMue | dimitern: I'm just doing, but I would like you to read the doc (and the according code) too. it is a bit more coding, but not so very much. and the test suites per method can be wonderfully reused | 08:21 |
| === psivaa_ is now known as psivaa_-afk | ||
| TheMue | dimitern: yeah, I needed it to inject the factory for the API version I want to test | 08:21 |
| dimitern | TheMue, the problem is, you can't pass a constructor returning a concrete type (or pointer thereof) | 08:21 |
| dimitern | TheMue, i.e. firewaller.NewFirewallerAPI(...) (*State, error) | 08:22 |
| TheMue | dimitern: that's why I had to return the interface | 08:22 |
| TheMue | dimitern: and I'm wrapping it in a little func with the signature above | 08:22 |
| TheMue | dimitern: here the static typing hinders us to use the factory directly *sigh* | 08:23 |
| TheMue | dimitern: never tried "type Factory func(...) (interface{}, error)" and then cast "firewaller.NewFirewallerAPI.(Factory)" | 08:24 |
| dimitern | TheMue, won't work - see http://play.golang.org/p/9UQE8Fh2Co | 08:26 |
| TheMue | dimitern: yeah, just tested too. would have been nice | 08:28 |
| === psivaa_-afk is now known as psivaa | ||
| wallyworld | TheMue: i think you're working on this - I have an existing test suite where I want to change the result of RegisterStandardFacade() ie change the version registered. But calling this method a second time gives an error. What's the way we're to use to change the result of BestAPIVersion() for testing? Or even unregister a facade for testing? | 08:34 |
| wallyworld | I need to do both | 08:35 |
| TheMue | wallyworld: it's not yet proposed, I had it in a branch I pulled back but it will come with the next one: a function to de-register an API returning a restore function to call with defer | 08:36 |
| wallyworld | ok, will that land soon? | 08:37 |
| TheMue | wallyworld: it has to be called before the API is connected, so that the resolving of the available APIs doesn't return it | 08:37 |
| TheMue | wallyworld: think so, yes, but I can already point you to it. it's a small one | 08:38 |
| TheMue | wallyworld: one moment, will find the code | 08:38 |
| wallyworld | ok | 08:38 |
| wallyworld | although if it's in common infrastructure it's best to wait | 08:38 |
| wallyworld | but i do need to be able to land my branch tomorrow hopefully | 08:38 |
| TheMue | wallyworld: take a look here, it's replacing an existing func in common.FacadeRegistry: https://github.com/TheMue/juju/compare/capability-detection-for-networker#diff-dfdd9146133bcedaf779a17388c5a27fR234 | 08:40 |
| wallyworld | ok, thank you | 08:40 |
| TheMue | wallyworld: and here I used it https://github.com/TheMue/juju/compare/capability-detection-for-networker#diff-13fd488899c8ceea9cc4183707b0b270R200 | 08:41 |
| dimitern | TheMue, you can do a wrapper func taking any callable and returning something with (interface{}, error) - see http://play.golang.org/p/ZOqBHnjfyi | 08:41 |
| wallyworld | TheMue: thanks, looks like Discard is already in the code base | 08:42 |
| wallyworld | i might try and use it | 08:42 |
| TheMue | wallyworld: yes, it's only that the current Discard() cannot restore, that's all ;) | 08:43 |
| wallyworld | ah | 08:43 |
| TheMue | dimitern: yeah, adding this to our testing package simplifies the injection, nice | 08:43 |
| dimitern | TheMue, well, this is just a quick sketch, but WrapNew should do more checks and take a gc.C as well, but the idea is nice | 08:44 |
| dimitern | hey davecheney | 08:45 |
| TheMue | dimitern: yes, to be part of a testing package it has to | 08:51 |
| * TheMue just fetched a fresh coffee | 08:51 | |
| TheMue | still a bit tired and having aching legs after the visit of the Photokina fair yesterday. but has been fantastic, with many ideas where I'll spend my money for :D | 08:53 |
| wallyworld | jam: i pushed some changes. i used common.Facades.Discard("HighAvailability", 1) to remove the facade registration and so force the client caller to go through the older client facade | 09:36 |
| tasdomas | should I close the reviewboard review once it gets a "ship it!"? | 09:50 |
| jam | wallyworld: I think you have 1 typo, but otherwise LGTM | 09:58 |
| wallyworld | jam: ty | 09:58 |
| voidspace | hmmm... so I have a function that after a replicaset reconfig connects to all members of the replicaset | 10:01 |
| voidspace | and waits for *them all* to report a healthy status for all the other members | 10:01 |
| voidspace | so we know they're all up and they all know about the other members | 10:01 |
| voidspace | And the test *still* fails with this immediately after that check | 10:02 |
| voidspace | Message:"exception: need most members up to reconfigure, not ok : localhost:33975", Assertion:false} | 10:02 |
| voidspace | that just seems insane | 10:02 |
| voidspace | I'm going back to mongo support channels | 10:03 |
| TheMue | dimitern: so, took a deeper look at the server-side versioning and the tests. wrote some notes in the review. | 10:19 |
| dimitern | TheMue, cheers, will have a look | 10:23 |
| TheMue | dimitern: I'm currently thinking about mixing our ideas | 10:24 |
| dimitern | TheMue, I'm in process of changing my PR to use per-method suites to see how it will look like | 10:25 |
| TheMue | dimitern: ah, we're getting closer | 10:28 |
| aznashwan | hello, could somewhat please tell me what email/password rbttools expects of me? | 10:37 |
| aznashwan | it says in the documentation that it's the ones of the rbt server, and I presumed it was the github ones seeing as though we used 0auth; but those don't seem to work D: | 10:38 |
| davecheney | aznashwan: it is amazingly | 10:41 |
| davecheney | $YOURUSERNAME | 10:41 |
| davecheney | password: gh-oauth | 10:41 |
| === fabrice is now known as fabrice|lunch | ||
| davecheney | aznashwan: sorry, i think the password is oauth:<$YOURGITHUBUSERNAME> | 10:45 |
| jam | voidspace: standup? | 10:46 |
| voidspace | jam: oops, omw | 10:48 |
| aznashwan | davecheney: I've been spamming the server with requests for a while now, and still no luck D: | 10:51 |
| aznashwan | any chance the upload diff file directly to the webpage will work? | 10:52 |
| davecheney | aznashwan: you can do that | 10:57 |
| davecheney | but it needs to be a "full diff" | 10:57 |
| davecheney | the diff from github won't work | 10:57 |
| davecheney | :( | 10:57 |
| davecheney | i already tried that | 10:57 |
| davecheney | aznashwan: don't tell anyone, but if you just do a pull reequest on github | 10:58 |
| davecheney | then i'll review it and we don't need to worry about review board | 10:58 |
| aznashwan | davecheney: thanks a lot, I've already got two up now, and they've been looked into a little already, minor fixes each, freshly rebased on master so it will just be fast-forwards if they get merged | 11:00 |
| perrito666 | morning | 11:02 |
| davecheney | aznashwan: can you paste the urls | 11:08 |
| davecheney | ? | 11:09 |
| aznashwan | davecheney: sure, sorry: https://github.com/juju/juju/pull/748 https://github.com/juju/juju/pull/704 | 11:09 |
| aznashwan | davecheney: thanks for the reviews, will get to fixing the PR's immediately :D | 11:16 |
| davecheney | aznashwan: it's way past end of day for me | 11:16 |
| davecheney | i'll catch you on the flip slide | 11:17 |
| davecheney | side | 11:17 |
| aznashwan | davecheney: k, have a nice one :D | 11:17 |
| perrito666 | see you tonight davecheney | 11:18 |
| perrito666 | some times this irc channel feels like a relay race | 11:19 |
| davecheney | perrito666: we develop juju 24/7 | 11:21 |
| aznashwan | perrito666: a rolling stone gathers no moss | 11:22 |
| aznashwan | luckily, I ran out of things to bother you guys with, at least for the time being :v | 11:23 |
| perrito666 | aznashwan: I strongly disagree http://cdn2-b.examiner.com/sites/default/files/styles/image_content_width/hash/49/b3/49b30d7598f1527927df8a4cbd98e253.jpg?itok=AIdW4EIi | 11:23 |
| * voidspace lunch | 11:52 | |
| mattyw | perrito666, nice | 11:56 |
| perrito666 | mattyw: I was hoping someone would get the joke | 11:57 |
| perrito666 | .p | 11:57 |
| perrito666 | :p | 11:57 |
| mattyw | perrito666, I wouldn't go as far as to say joke ;) | 11:57 |
| perrito666 | o cmon, rolling stone, moss, picture, instant fun | 11:57 |
| mattyw | that must be some strange usage of the word fun I wasn't previously aware of | 12:03 |
| === fabrice|lunch is now known as fabrice | ||
| perrito666 | mattyw: arent you brittish? | 12:05 |
| mattyw | perrito666, I am, for now | 12:06 |
| perrito666 | :p then you are in no moral grounds to judge about fun then, we actually catalog brittish commedy films in a separate category :p | 12:06 |
| perrito666 | mattyw: moving out? | 12:06 |
| mattyw | perrito666, until Scotland gets independance | 12:07 |
| perrito666 | mattyw: ahh moving out the entire country, very cool move :p | 12:07 |
| mgz | 'd still be british surely... | 12:08 |
| mgz | mattyw: what did you want me for yesterday evening btw? | 12:08 |
| mattyw | mgz, I was getting crazy errors from the landing bot - seemed to suggested something was up with the server rather than the bot | 12:09 |
| mattyw | mgz, but it's fine now | 12:09 |
| mattyw | mgz, thanks anyway | 12:09 |
| mgz | I'll have a look at the logs | 12:09 |
| mattyw | mgz, here's one http://juju-ci.vapour.ws:8080/job/github-merge-juju/657/console | 12:10 |
| mgz | ta | 12:10 |
| perrito666 | its interesting, school here actually teaches kids that england, scotland and ireland+nort ireland as 3 separate countries and makes no mention whatsoever to the united kingdom until high school (near the end of it) | 12:11 |
| perrito666 | what would happen if there was a command clashing with a plugin? for intance if I was to create a charm command? | 12:11 |
| fabrice | perrito666: in France england scotland wales are the same and we always use england for uk | 12:12 |
| === Ursinha is now known as Ursinha-afk | ||
| mattyw | folks - after we land a branch reviewed on review board do we just mark it as close -> submitted? | 12:19 |
| mattyw | ^^ yes appears to be the answer | 12:19 |
| === Ursinha-afk is now known as Ursinha | ||
| axw | wallyworld: if/when my branch lands, and backups move off provider storage, I believe we can remove Storage from the Environ interface | 12:25 |
| wallyworld | yup \o/ | 12:25 |
| axw | just did a ~100line change which looks like it'll work | 12:25 |
| wallyworld | whoo hoo | 12:25 |
| axw | mattyw: that's what I did | 12:26 |
| axw | dunno if we're supposed to or not, but I guess so | 12:26 |
| wallyworld | i did that too | 12:27 |
| tasdomas | what package should I place a commonly used function in juju? | 12:31 |
| tasdomas | thinking of moving NowToTheSecond out of state | 12:31 |
| tasdomas | it's used both in tests and in production code, so I can't just put it in a testing package | 12:31 |
| perrito666 | axw: tell me more about backups | 12:33 |
| TheMue | hmm, not yet friend with rbt. if it tells me "ERROR: There don't seem to be any diffs!" when calling rbt post, how can I still post my pull request? | 12:53 |
| perrito666 | TheMue: did you do the upstream configuration magic menn0 suggested? | 12:54 |
| TheMue | perrito666: I'm just following the described workflow in the mail | 12:56 |
| TheMue | perrito666: which "magic" (and why do we need magic when simple tools should do it)? | 12:56 |
| perrito666 | TheMue: I meant a configuration in your .Idontrememberwhatrc | 12:58 |
| TheMue | perrito666: I have no .Idontrememberwhatrc *lol* | 12:59 |
| TheMue | perrito666: hehe, no, will take a closer look | 12:59 |
| perrito666 | TheMue: apologies, I do not recall the whole thread | 13:01 |
| TheMue | perrito666: but I found the missing line, and no need to appologize, it's my fault to not configure it correctly | 13:02 |
| TheMue | perrito666: but now added the magic line and it ask me to authenticate. so getting closer, yeah. | 13:03 |
| perrito666 | TheMue: ah apparently you ned to user | 13:03 |
| perrito666 | agh | 13:03 |
| perrito666 | need to use username@github | 13:03 |
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| mattyw | axw, cool thanks | 13:09 |
| mattyw | axw, I saw that's what you did so that's what I went for | 13:10 |
| TheMue | perrito666: this tool hates me, cannot login | 13:18 |
| * perrito666 tried to do a portmanteau between troll and tool but feared to accidentally post a profanity in English | 13:19 | |
| TheMue | ;) | 13:19 |
| TheMue | I tried with TheMue as well as my E-Mail as username, but still cannot login | 13:20 |
| === mbruzek is now known as mbruzek_ | ||
| === mbruzek_ is now known as mbruzek | ||
| TheMue | Changing the password just to make sure doesn't help too. *sniff* | 13:35 |
| TheMue | What.does.this.tool.want.from.me? | 13:35 |
| perrito666 | TheMue: your soul | 13:39 |
| perrito666 | what are you using for a user? | 13:39 |
| natefinch | you guys talking about rbt? | 13:40 |
| perrito666 | TheMue: the logs show ericsnow suggesting: | 13:40 |
| perrito666 | to katco "oauth:katco-@github" | 13:40 |
| natefinch | right, I was going to say that | 13:41 |
| natefinch | for some reason eric set all our passwords to that ;) | 13:41 |
| TheMue | ahh, wil try | 13:42 |
| natefinch | (joking of course, you should use oath:<github_username>@github as your password | 13:42 |
| TheMue | perrito666: thanks, now it works *phew* you saved my soul *grin* | 13:43 |
| perrito666 | natefinch: well makes sense, passwords are most likely hashAlgorythm:hash and so oauth: must trigger a django auth plugin if I understand correctly that makes the authorization dance | 13:45 |
| TheMue | ok, now it would be nice if markdown files could be directly rendered like in github | 13:46 |
| === fabrice is now known as fabrice|afk | ||
| wwitzel3 | natefinch: when you have a chance, http://reviews.vapour.ws/r/42/ | 14:21 |
| natefinch | wwitzel3: cool, I'll take a look | 14:22 |
| wwitzel3 | natefinch: it is the changes from that ML thread on rsyslod | 14:22 |
| aznashwan | could someone please tell what the hell is rbt post expecting as a username/password? | 14:39 |
| ericsnow | username: <github username> password: "oauth:<github username>@github" | 14:40 |
| === jheroux_away is now known as jheroux | ||
| natefinch | ericsnow: can you just email that out to the list as a one line email? | 14:47 |
| ericsnow | natefinch: good idea :) | 14:48 |
| === fabrice|afk is now known as fabrice | ||
| TheMue | ericsnow: when publishing changes after an "rbt post -u" I currently get a HTTP 500. known problem? | 14:57 |
| ericsnow | TheMue: looks like the revision you are trying to post is already associated with a different review request | 14:58 |
| TheMue | ericsnow: eh, I accidently posted it without -u but immediately discarded it | 14:59 |
| ericsnow | TheMue: each review request has an associated repo revision (which has a UNIQUE constraint) | 15:00 |
| TheMue | ericsnow: as I said, it's discarded. so the software should not care | 15:00 |
| ericsnow | TheMue: unfortunately discarded requests still keep that revision locked up | 15:00 |
| ericsnow | TheMue: it's dumb | 15:00 |
| ericsnow | TheMue: I can "permanently delete" that accidental request if you tell me the number | 15:01 |
| TheMue | ericsnow: so HTTP 500 tells me "Hey you dumb guy, make another change, push it, and post it again."? ;) | 15:01 |
| ericsnow | TheMue: I expect that this is going to keep happening to people | 15:01 |
| ericsnow | TheMue: exactly :) | 15:02 |
| TheMue | ericsnow: hehe | 15:02 |
| TheMue | ericsnow: but deletion is better, it's the 44 | 15:02 |
| perrito666 | natefinch: wwitzel3 stdup? | 15:02 |
| ericsnow | TheMue: I expect we need to either come up with a github subcommand that wraps rbt intelligently or get github-reviewboard integration rolled out | 15:03 |
| natefinch | perrito666, wwitzel3, ericsnow: would you guys mind if we delayed until later? I have a sleeping child next to me, and I'd rather not wake her | 15:03 |
| ericsnow | TheMue: I expect we'll do the latter at some point (hopefully in the near future) regardless | 15:03 |
| perrito666 | natefinch: np | 15:04 |
| ericsnow | natefinch: let a sleeping child lie :) | 15:04 |
| voidspace | coffee | 15:05 |
| TheMue | ericsnow: a seamless integration would be nice, indeed | 15:05 |
| ericsnow | TheMue: I've deleted 44 so you should be good to try "rbt post -u" again. | 15:06 |
| TheMue | ericsnow: great, will do, thanks | 15:07 |
| ericsnow | TheMue: np, sorry for the trouble | 15:07 |
| dimitern | TheMue, perrito666, updated http://reviews.vapour.ws/r/33/ care to take a final look? | 15:15 |
| perrito666 | dimitern: did you see davecheneys comment? | 15:15 |
| TheMue | dimitern: already started | 15:16 |
| dimitern | perrito666, I did change back the tag fields as they were | 15:16 |
| TheMue | dimitern: regarding the order inside the file, currently the declarations of base and V0 as well as methods of them are in a mixed order. I only meant to write all base definitions first and all V0 definitions then, simply to group them properly | 15:18 |
| dimitern | TheMue, you mean have all func (f *FirewallerAPIBase) Methods() then the FirewallerAPIV0 ones? | 15:24 |
| TheMue | dimitern: yep | 15:24 |
| dimitern | TheMue, ok, I misunderstood then :) will do and post an updated diff | 15:25 |
| TheMue | dimitern: thanks | 15:25 |
| TheMue | dimitern: btw, in your mail, you meant "an" agent API, and not "the" agent API, don't you? not that I'm getting you wrong here. | 15:27 |
| dimitern | TheMue, updated | 15:28 |
| dimitern | TheMue, yes, "an agent api", the AgentAPI (perhaps poorly named) is just one facade | 15:29 |
| TheMue | dimitern: fine | 15:29 |
| TheMue | dimitern: for the testing approach I still would like to know how we'll test a func introduced in V1 in V2 then? | 15:31 |
| TheMue | dimitern: but I've got an idea here | 15:31 |
| TheMue | dimitern: currently your base is a type with methods | 15:32 |
| wwitzel3 | I see we have juju/utils/set and it currently has implemented Strings | 15:32 |
| dimitern | TheMue, le me think a bit | 15:32 |
| TheMue | dimitern: how about changing it into a simple struct which is passed to simple test functions as a first argument | 15:32 |
| dimitern | TheMue, so you have Foo() in V1, which is unchanged in V2? | 15:33 |
| TheMue | dimitern: so for V0 I implement the V0 test functions and use them in the test suite | 15:33 |
| wwitzel3 | my question is, if I want to have a set of tags, should I implement that in utils/set and make it have a Tags implementation? | 15:33 |
| TheMue | dimitern: in V1 I only write the new test functions and write my test suite V1 like you've done using these functions too | 15:34 |
| dimitern | TheMue, the we can use testFoo(c, patchFunc(taking NewAPIV1 in v1_test and NewAPIV2 in v2_test) | 15:34 |
| TheMue | dimitern: and so in with the next functions | 15:34 |
| dimitern | TheMue, more or less yes | 15:34 |
| dimitern | TheMue, I'm not saying "this is the way we should test API versions, period." :) | 15:35 |
| wwitzel3 | because right now, I'm converting from user input string, to tag, using the tag.String() method with set to create a unique set of tags, then converting them back to tags with ParseTag as I iterate over them | 15:35 |
| TheMue | dimitern: hehe | 15:35 |
| dimitern | TheMue, it's just we're looking for the best way, and I don't think we're quite there yet | 15:35 |
| TheMue | dimitern: no, me neither, only thinking of a good solution too | 15:35 |
| dimitern | TheMue, but for the time being, I'd rather move on with the ongoing tasks and revisit testing later | 15:35 |
| wwitzel3 | my thought is, since it lives under juju/utils already, our set implementation knowing about types from juju/names seems ok | 15:35 |
| wwitzel3 | thoughts? | 15:36 |
| TheMue | dimitern: so your approach, but with no real base in the sense of inheritence, more as a growing set of versioned test functions used in the individual suites | 15:36 |
| TheMue | dimitern: ok | 15:36 |
| dimitern | TheMue, If we agree on an approach, I promise to take some time to refactor existing facade tests | 15:36 |
| wwitzel3 | TheMue: I will act as a witness along with the IRC logs of dimitern's promise ;) | 15:36 |
| TheMue | dimitern, wwitzel3: *rofl* | 15:37 |
| dimitern | wwitzel3, +1 :) | 15:37 |
| dimitern | wwitzel3, that's a very good idea IMO | 15:37 |
| dimitern | wwitzel3, having set.NewTags() etc. so we can use them with names.Tag and derived types, store them as tags, but have the same UX as with Strings | 15:38 |
| TheMue | dimitern: I'll note a quick dummy containing your approach and my latest thoughts. think it's a good way, flexible and less boilerplate | 15:39 |
| dimitern | wwitzel3, I'd rather have map[names.Tag]bool as internal implementation for set.Tags, but optionally, you can have NewTags(names.Tag...) and NewTagsFromStrings(string...) internally using ParseTag on each | 15:40 |
| dimitern | TheMue, cheers :) | 15:40 |
| * dimitern has reached eod | 15:45 | |
| natefinch | dimitern, TheMue, rogpeppe1, mgz: do you guys know offhand if we try to store charm configuration keys in mongo as key names? | 15:46 |
| rogpeppe1 | natefinch: i think we *did* do that AFAIR | 15:46 |
| natefinch | rogpeppe1: hmm interesting | 15:47 |
| natefinch | rogpeppe1: there's an email on the juju ubuntu list entitled "Regression with dots in charm options" | 15:48 |
| mgz | natefinch: yeah, saw that, bug 1308146 being the relevent thing | 15:49 |
| mup | Bug #1308146: not okForStorage error when deploying local charm <charms> <deploy> <juju-core:Triaged> <https://launchpad.net/bugs/1308146> | 15:49 |
| mgz | natefinch: what's not clear to me is when then actually started breaking | 15:50 |
| mgz | I guess just in 1.16->1.18? | 15:50 |
| natefinch | yeah, could be. I care slightly less about when than about why | 15:51 |
| dimitern | natefinch, we don't do that without escaping $ and . | 15:51 |
| rogpeppe1 | natefinch: looking at state/settings.go, it does at least try to replace the dot (with full-width unicode dot) | 15:51 |
| natefinch | dimitern: I had hoped we sanitized it | 15:51 |
| dimitern | natefinch, take a look at state/settings.go | 15:51 |
| === fabrice is now known as fabrice|family | ||
| dimitern | natefinch, do you know what this job opening is for ? https://ch.tbe.taleo.net/CH03/ats/careers/requisition.jsp?org=CANONICAL&cws=1&rid=830 (i.e. juju as in core or as in ecosystem or something else?) | 15:53 |
| dimitern | a good friend of mine (also good dev) called to ask if we're hiring | 15:54 |
| voidspace | mgz: ping | 15:54 |
| natefinch | dimitern: we have 1 engineer position open and that one.... that one you linked to is kind of more of a do a little bit of everything job to work with kapil and mark S and mark ramm etc for like product maangement type stuff | 15:55 |
| natefinch | dimitern: so, that one is higher up in juju, but there is a juju core position open IIRC | 15:55 |
| dimitern | natefinch, I see, well he's based in BG, so I guess no juju-core openings at the moment? | 15:56 |
| dimitern | natefinch, I'd appreciate if you find it and send me the link by email, as browsing all openings I couldn't find juju-specific openings except this one | 15:57 |
| mgz | voidspace: hey | 15:57 |
| natefinch | dimitern: I think the location is just a suggestion. Definitely ping Alexis about it, I know we have a dev position open, and I'm pretty sure it's in core. | 15:58 |
| voidspace | mgz: hey, hi | 15:58 |
| voidspace | mgz: but never mind... | 15:58 |
| mgz | ;_; | 15:58 |
| voidspace | mgz: unping :-) | 15:58 |
| dimitern | natefinch, cheers, will do | 15:58 |
| wwitzel3 | dimitern: thanks :) | 16:00 |
| perrito666 | dimitern: that is perhaps the most generic role description I have seen in my whole life :p | 16:00 |
| dimitern | perrito666, lol, most of these on taleo we have are like this | 16:01 |
| === Ursinha is now known as Ursinha-afk | ||
| voidspace | ericsnow: ping | 16:06 |
| ericsnow | voidspace: hey hey hey | 16:06 |
| voidspace | ericsnow: I just failed to create a review board review request | 16:06 |
| voidspace | ericsnow: or at least, I created one entirely unrelated to my branch | 16:06 |
| voidspace | ericsnow: could you walk me through it | 16:06 |
| voidspace | ericsnow: I have a github pull request for the branch | 16:06 |
| ericsnow | voidspace: what is the review number? | 16:07 |
| voidspace | ericsnow: https://github.com/juju/juju/pull/776 | 16:07 |
| voidspace | ericsnow: 45 | 16:07 |
| ericsnow | voidspace: and it is associated with revision 9cac03a, right? | 16:07 |
| voidspace | ericsnow: right | 16:08 |
| ericsnow | voidspace: did you set TRACKING_BRANCH = "upstream/master" in ~/.reviewboardrc? | 16:08 |
| voidspace | ericsnow: no... | 16:08 |
| voidspace | ericsnow: I'll do that | 16:09 |
| voidspace | ericsnow: should that change be enough? | 16:09 |
| ericsnow | voidspace: if origin is set to something else (which it should be) and master is not up to date with upstream, you'll get bad diffs | 16:09 |
| ericsnow | voidspace: yeah | 16:09 |
| voidspace | ericsnow: I made sure my branch was up to date first | 16:09 |
| voidspace | ericsnow: so how do I kill that request and re-create it? | 16:10 |
| ericsnow | voidspace: well, give the tracking branch thing a try | 16:10 |
| voidspace | I've done that, do I just post again? | 16:10 |
| ericsnow | voidspace: reviewboard is kind of dumb in that regard | 16:10 |
| ericsnow | voidspace: you have to do "rbt post -u" or "rbt post -r #" | 16:10 |
| ericsnow | voidspace: since RB links review requests to revisions uniquely and even discarded requests count :( | 16:11 |
| ericsnow | voidspace: just doing rbt post would give you an oh-so-helpful 500 error | 16:11 |
| voidspace | ericsnow: so the diff is now correct - but title and summary are still the old ones | 16:13 |
| voidspace | I did "rbt post -u" | 16:13 |
| ericsnow | voidspace: cool | 16:13 |
| ericsnow | voidspace: just edit them in the web interface | 16:13 |
| voidspace | if I do "-r #" will it create a new one? | 16:13 |
| voidspace | ok | 16:13 |
| voidspace | ericsnow: thanks for your help | 16:13 |
| voidspace | katco: I think you're an OCR today | 16:16 |
| voidspace | reviews.vapour.ws/r/45/ | 16:16 |
| katco | voidspace: right you are. currently reviewing a largish change-set | 16:16 |
| voidspace | katco: ok, cool | 16:17 |
| voidspace | :-) | 16:17 |
| ericsnow | voidspace: -r just means update that specific review request and -u mean match the revision to a request and update that one | 16:17 |
| ericsnow | voidspace: glad to help! | 16:17 |
| voidspace | ericsnow: ah right | 16:17 |
| voidspace | ericsnow: much appreciated :-) | 16:17 |
| voidspace | looking forward to seeing you all again soon | 16:17 |
| ericsnow | voidspace: same here :) | 16:21 |
| === jog_ is now known as jog | ||
| voidspace | ericsnow: so in summary, it should definitely be done - but I shouldn't do it... | 17:06 |
| === psivaa_-afk is now known as psivaa_ | ||
| voidspace | ericsnow: and what do you mean "not sure if I intended this change", after you told me to do it... | 17:06 |
| voidspace | ericsnow: :-p | 17:06 |
| ericsnow | voidspace: :) just not in that PR | 17:06 |
| voidspace | ericsnow: heh, ok | 17:07 |
| voidspace | ericsnow: one for you then | 17:19 |
| voidspace | ericsnow: http://reviews.vapour.ws/r/46/ | 17:19 |
| ericsnow | voidspace: that might take a while to review :) | 17:20 |
| voidspace | ericsnow: let me know when you can get to it | 17:21 |
| voidspace | I've got all night... | 17:21 |
| ericsnow | voidspace: done | 17:23 |
| ericsnow | voidspace: I also added a little more explanation to the PR summary (which will become the merge commit message, right?) | 17:23 |
| ericsnow | voidspace: if you don't like it feel free to edit or remove it :) | 17:24 |
| voidspace | ericsnow: looks good to me | 17:28 |
| voidspace | ericsnow: thanks | 17:28 |
| ericsnow | voidspace: I was creating the same PR and that was my summary :) | 17:28 |
| voidspace | Heh | 17:29 |
| voidspace | ericsnow: I'm sorry... | 17:29 |
| ericsnow | voidspace: don't be :) | 17:29 |
| voidspace | ericsnow: I'll set the issue on review 45 to fixed as it is no longer a diff against upstream/master | 17:30 |
| voidspace | or at least won't be shortly | 17:30 |
| ericsnow | voidspace: totally | 17:31 |
| voidspace | and that's me done for the day | 17:31 |
| voidspace | see you all tomorrow | 17:31 |
| ericsnow | voidspace: see ya | 17:31 |
| katco | holy mother of god there's a second page to this diff. | 17:37 |
| natefinch | lol | 17:37 |
| katco | <-- first time being OCR. gets thrown into the deep end immediately. | 17:38 |
| ericsnow | katco: looking at one of mine? <wink> | 17:39 |
| katco | lol no, axw's. | 17:39 |
| === Ursinha-afk is now known as Ursinha | ||
| sinzui | natefinch, can you get someone to look into bug 1370635 | 18:01 |
| mup | Bug #1370635: Unable to connect to environment after local upgrade on precise <ci> <precise> <regression> <upgrade-juju> <juju-core:Triaged> <https://launchpad.net/bugs/1370635> | 18:01 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1370635 | ||
| natefinch | sinzui: yep. | 18:03 |
| === niedbalski_ is now known as niedbalski | ||
| aisrael | Has relation-list been deprecated? | 18:47 |
| aisrael | It's being referenced from a charm MP I'm reviewing, but I only see that command available in the juju-0.7 package | 18:48 |
| === Ursinha is now known as Ursinha-afk | ||
| === fuzzy_ is now known as Ponyo | ||
| === Ursinha-afk is now known as Ursinha | ||
| === bodie_ is now known as Guest20692 | ||
| === Guest20692 is now known as bodie_ | ||
| katco | marcoceppi: ping | 20:00 |
| sinzui | 1.21-alpha1 is released and it uses devel streams instead of the released streams | 20:07 |
| === Ursinha is now known as Ursinha-afk | ||
| perrito666 | natefinch: why is there only one of those bugs you emailed on the topic? | 20:21 |
| natefinch | perrito666: not sure what you're talking anout | 20:26 |
| natefinch | I really have to run, sorry | 20:27 |
| === Ursinha-afk is now known as Ursinha | ||
| === jheroux is now known as jheroux_away | ||
| menn0 | waigani: morning | 21:08 |
| waigani | menn0: hey | 21:11 |
| menn0 | did you see the CI blocker: bug 1370635 | 21:12 |
| mup | Bug #1370635: Unable to connect to environment after local upgrade on precise <ci> <precise> <regression> <upgrade-juju> <juju-core:Triaged> <https://launchpad.net/bugs/1370635> | 21:12 |
| menn0 | just looking at the details on the ticket it seems like it might be related to recent MESS related changes | 21:12 |
| menn0 | waigani: what do you think? | 21:12 |
| waigani | menn0: yeah, what is this: - '[fe80::f816:3eff:fe1f:67e1]:17070' | 21:12 |
| waigani | looks like a mac address? | 21:13 |
| menn0 | it's a IPv6 address | 21:13 |
| menn0 | get used to them :) | 21:13 |
| waigani | oooh | 21:14 |
| waigani | menn0: so you can't connect to the API if the user is in state but not added to the environment | 21:18 |
| menn0 | yep | 21:18 |
| menn0 | waigani: perhaps one of the migration steps isn't doing the right thing? | 21:19 |
| waigani | that would be the first thing I'd check, but he's authing with admin and I'd be surprised if admin was not added as a user to the env | 21:19 |
| menn0 | waigani: thumper is out today isn't he? | 21:19 |
| waigani | having said that, thumper has been working hard to 'demote' the status of the admin user - and we do not have an admin user by default in an env | 21:20 |
| waigani | menn0: yep, thumper is away | 21:20 |
| menn0 | waigani: true. but I don't think those changes are in 1.21-alpha2.1 are they? | 21:21 |
| waigani | yeah, no but didn't he land some prelim branches? | 21:21 |
| menn0 | waigani: I'm just in the middle of testing my last bit of upgrade sync changes (yay!) and then I can jump on this blocker | 21:21 |
| menn0 | waigani: it seems like it won't be too hard to figure out | 21:21 |
| waigani | anyway, it's just a hunch but I'd check if the user is in the environment | 21:21 |
| menn0 | (famous last words) | 21:22 |
| waigani | yeah, sigh | 21:22 |
| === Ursinha is now known as Ursinha-afk | ||
| perrito666 | ok, lets try the new propose thing | 22:04 |
| === kwmonroe_ is now known as kwmonroe | ||
| perrito666 | TheMue: still around? | 22:09 |
| menn0 | davecheney, waigani: thumper is out today. email standup? | 22:50 |
| davecheney | menn0: sgtm | 22:56 |
| perrito666 | hey, someone is pushing without previously checking their code | 22:59 |
| perrito666 | state/metrics.go:231: no formatting directive in Warningf call | 22:59 |
| perrito666 | worker/uniter/charm/bundles_test.go:120: arg apiCharm.ArchiveURL() for printf verb %q of wrong type: (*net/url.URL, github.com/juju/utils.SSLHostnameVerification, error) | 22:59 |
| menn0 | perrito666: I don't think everyone has the pre-commit hook in place | 23:06 |
| menn0 | perrito666: so they don't see the vet warning | 23:06 |
| perrito666 | menn0: well in this case the push hook did not fail, it just warned me | 23:06 |
| menn0 | perrito666: yep I get it too. I don't think the hook is set up to fail on vet errors, just warn as it does. | 23:07 |
| perrito666 | menn0: odd, I had the impression it failed | 23:07 |
| * menn0 shrugs | 23:08 | |
| menn0 | perrito666: I've only seen it warn so far (I've seen other vet warnings in the past) | 23:08 |
| perrito666 | ¯\_(ツ)_/¯ | 23:08 |
| perrito666 | sweet not only github suggests me to PR my recently pushed branch, it also shows a warning with a link to contributing suggesting to review the contributor policy of this repo | 23:09 |
| perrito666 | sweeet http://reviews.vapour.ws/r/49/diff/# | 23:13 |
| davecheney | perrito666: review now, lots of comments | 23:21 |
| perrito666 | oh cool, tx davecheney | 23:22 |
| perrito666 | davecheney: reviewboard does not seem to agree | 23:23 |
| davecheney | publised | 23:24 |
| perrito666 | aghh replying takes me out of the diff | 23:26 |
| davecheney | it's a brilliant system | 23:26 |
| davecheney | i love how thre are two different ways to publish your comments | 23:27 |
| davecheney | one, with the "review" button | 23:27 |
| davecheney | which allows you to add a final comment | 23:27 |
| davecheney | and the other, the yelow hovering bar at the top of the screen which does not let you add comments | 23:27 |
| * perrito666 goes read the annotatef godoc after an intriguing comment | 23:28 | |
| davecheney | pretty sure | 23:29 |
| davecheney | errors.Annotatef(nil, "something") => nil | 23:29 |
| perrito666 | excelent, I just read | 23:29 |
| davecheney | if it doesn't then ignore my comemnt | 23:29 |
| === Ursinha-afk is now known as Ursinha | ||
| perrito666 | yup // If the error argument is nil, the result of Annotatef is nil. | 23:30 |
| menn0 | davecheney, perrito666: it does. errors.Trace too. | 23:30 |
| perrito666 | but isnt it a bit anti idiomatic if I return (Whatever, err) instead of an explicit nil? | 23:31 |
| perrito666 | and for the matter an explicit zeroed Whatever when err != nil? | 23:31 |
| perrito666 | davecheney: tx, Ill fix those while the food is in the oven | 23:32 |
| perrito666 | you people make excelent company while making dinner everyday :p | 23:33 |
| menn0 | perrito666: you're right. if you're returning an error it's better to return nil for the other return value if there is one. | 23:34 |
| menn0 | that Annotate/Trace feature is best used for funcs that just return an error | 23:35 |
| menn0 | perrito666: what's for dinner? | 23:35 |
| menn0 | waigani: the upgrade steps aren't even running | 23:36 |
| waigani | O_o ? | 23:36 |
| menn0 | waigani: I think what's happening is a chicken and egg scenario | 23:36 |
| waigani | because upgrades needs access to the API? | 23:36 |
| menn0 | waigani: the upgrade steps infrastructure wants an api.State connection | 23:36 |
| menn0 | waigani: to give to the upgrade steps | 23:37 |
| menn0 | waigani: but it can't open one, because the upgrade steps haven't run yet | 23:37 |
| menn0 | waigani: fucking awesome | 23:37 |
| menn0 | waigani: I'm not fully up to speed with why the API connection can't be opened though | 23:38 |
| waigani | but if the server is 1.20, a 1.21 client should still be able to connect to start the upgrades? | 23:38 |
| menn0 | waigani: starting the upgrade worked fine | 23:39 |
| menn0 | jujud restarted into 1.21 | 23:39 |
| menn0 | it gets to the point of running the upgrade steps | 23:39 |
| menn0 | and that's where things go wrong | 23:39 |
| waigani | ah, the order of the upgrade steps? | 23:39 |
| menn0 | so the server is no longer 1.20 | 23:39 |
| menn0 | waigani: nope | 23:39 |
| waigani | do we need to migrate the users to the env as the first step? | 23:40 |
| menn0 | before any upgrade steps are run an API connection for machine-0 is set up | 23:40 |
| perrito666 | davecheney: btw, partial review means there are more underway? | 23:40 |
| waigani | and machine-0 is still 1.20 at that point right? | 23:40 |
| menn0 | waigani: no | 23:40 |
| waigani | ah | 23:41 |
| menn0 | waigani: machine-0 is running 1.21 | 23:41 |
| menn0 | waigani: but the upgrade steps haven't run yet | 23:41 |
| waigani | well that's just stupid | 23:41 |
| menn0 | it might be that we need to run DB migrations first, they need a state.State which will still work | 23:42 |
| waigani | so we need to make an exception to the new auth rule to allow upgrade steps to complete | 23:42 |
| menn0 | but that's a fairly major change | 23:43 |
| menn0 | waigani: that's probably a wiser move | 23:43 |
| menn0 | waigani: I'm not completely up to speed with the recent user changes | 23:43 |
| menn0 | waigani: do you understand what "environment not found" means? | 23:44 |
| waigani | menn0: so we may have to stage the migration to env users across two versions. first upgrade ensures all local users (i.e. admin) are added to environment. Second upgrade locks down API connection to allow connections only by users added to the environment. | 23:46 |
| menn0 | waigani: that sounds sensible | 23:47 |
| menn0 | waigani: or have some kind of workaround to just allow the migrations to work and then lock down the API | 23:48 |
| waigani | menn0: yep, that is the basic idea - how we implement that is the question | 23:48 |
| perrito666 | menn0: omlette with onnions and what wallyworld calls capsicums and escalopes | 23:49 |
| * wallyworld doesn't know what an escalopes is | 23:50 | |
| menn0 | waigani: I've got to meet a friend for coffee | 23:50 |
| perrito666 | wallyworld: http://en.wikipedia.org/wiki/Schnitzel | 23:50 |
| menn0 | waigani: back in a bit | 23:51 |
| wallyworld | perrito666: oh, i know what Schnitzel is | 23:51 |
| waigani | menn0: one *horrible* workaround would be if user = "admin" all good | 23:51 |
| perrito666 | wallyworld: sorry translated spanish -> french -> english | 23:51 |
| perrito666 | :p | 23:51 |
| waigani | but we'd still have to stage the migration with a workaround | 23:51 |
| wallyworld | perrito666: sounds delicious, what time should i come over? | 23:52 |
| waigani | as the workaround would have to be removed from the code once all users are in the env | 23:52 |
| waigani | menn0: ^ | 23:52 |
| waigani | menn0: apiserver/admin.go:181 - this is where the error is coming from. | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
