| bigjools | thumper: is this likely to be fallout from your series changes? http://pastebin.ubuntu.com/5691125/ | 02:28 |
|---|---|---|
| bigjools | I am trying to work out if it's a bug in the core or in the maas provider | 02:29 |
| * thumper looks | 02:29 | |
| thumper | bigjools: yes, you want --fake-series=precise | 02:30 |
| bigjools | thumper: on the bootstrap command line? | 02:30 |
| thumper | aye | 02:30 |
| bigjools | ok cheers | 02:30 |
| thumper | as you are uploading tools | 02:30 |
| thumper | and by default, it makes tools for your series | 02:30 |
| thumper | but the machine you are booting says precise | 02:30 |
| bigjools | it worked \o/ | 02:31 |
| thumper | although you should have public tools for 1.9.13 now | 02:31 |
| bigjools | thumper: public? | 02:32 |
| thumper | as in, ec2 public bucket | 02:32 |
| bigjools | thumper: I am not using ec2 | 02:32 |
| bigjools | this is maas | 02:32 |
| * thumper nods, ok | 02:32 | |
| bigjools | anyway, bootstrap "completed" so thanks :) | 02:33 |
| bigjools | is there a local placement option in juju-core like there was in Python? | 04:36 |
| markramm | there is one under review right now | 04:36 |
| bigjools | ok ta | 04:37 |
| markramm | bigjools: it will function similarly to deploy-to but be called force-machine | 04:37 |
| markramm | discussion on the mailing list about it is ongoing | 04:37 |
| bigjools | markramm: I don't know much about this - is deploy-to a cmd line option? | 04:37 |
| davecheney | bigjools: no, that isn't an option at the moment | 04:39 |
| markramm | bigjools: it was a jitsu feature in python | 04:41 |
| markramm | that took a machine ID and installed a service on that particular machine | 04:41 |
| bigjools | ah ok, no wonder I hadn't heard of it | 04:42 |
| bigjools | thanks both | 04:42 |
| markramm | I just now realize that I'm not sure what you meant by "local placement" | 04:42 |
| markramm | so perhaps I got it wrong | 04:42 |
| bigjools | markramm: "placement: local" in the environment config | 04:43 |
| bigjools | juju-core complains if I have that in | 04:43 |
| davecheney | bigjools: yes, we don't understand that key | 04:45 |
| bigjools | davecheney: the error message was a little opaque: error: placement: expected nothing, got "local" | 04:46 |
| davecheney | yeah, that isn't that helpful | 04:46 |
| * bigjools files a bug | 04:46 | |
| davecheney | bigjools: good idea | 04:46 |
| bigjools | I was actually looking to avoid bringing up another machine in maas and re-using the bootstrap node | 04:46 |
| bigjools | since it's slow - there's no fast installer yet | 04:47 |
| bigjools | but no biggie | 04:47 |
| davecheney | bigjools: https://launchpad.net/ubuntu/raring/+source/mongodb/1:2.2.4-0ubuntu1 | 05:20 |
| davecheney | is it possible to drag this into a PPA targeting precise ? | 05:21 |
| bigjools | it might be, depends if it builds with precise's available deps | 05:21 |
| davecheney | should I try a source build on a Precise vm | 05:28 |
| davecheney | ? | 05:28 |
| * thumper goes to make dinner | 05:42 | |
| bigjools | davecheney: use pbuilder | 05:43 |
| davecheney | bigjools: speak to me as if I were a child | 05:43 |
| bigjools | well you can use a VM | 05:44 |
| bigjools | davecheney: I've got 4 naughty kids, don't tempt me! | 05:44 |
| davecheney | bigjools: if you want to come to NSW for a spankin' let me know | 05:44 |
| davecheney | i'm happy to arrange | 05:44 |
| bigjools | what, you'd whizz straight past me then | 05:45 |
| bigjools | so you can use a VM, or pbuilder automates an environment for a particular release | 05:45 |
| bigjools | whatever you're comfortable with | 05:45 |
| davecheney | i'll try pbuilder | 05:46 |
| davecheney | i expect to get screwed on permissings | 05:46 |
| davecheney | permissions | 05:46 |
| bigjools | frankly, I fire up a canonistack instance these days | 05:46 |
| bigjools | massively quicker | 05:47 |
| * davecheney deployes the ubuntu charm | 05:47 | |
| bigjools | just dget the source for raring on a precise instance | 05:47 |
| * bigjools afk | 05:50 | |
| rogpeppe | mornin' all! | 06:33 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: TheMue | Bugs: 0 Critical, 52 High - https://bugs.launchpad.net/juju-core/ | ||
| TheMue | moin moin btw | 07:31 |
| rogpeppe | TheMue: hiya | 08:11 |
| rogpeppe | TheMue: i believe you are the on-call reviewer today... | 08:11 |
| rogpeppe | TheMue: i have some reviews for you :-) | 08:11 |
| rogpeppe | TheMue: https://codereview.appspot.com/8458044/ https://codereview.appspot.com/8487044/ https://codereview.appspot.com/8533044/ https://codereview.appspot.com/8534043/ | 08:11 |
| rogpeppe | fwereade: if you fancy having a look, another opinion would be nice too | 08:12 |
| fwereade | rogpeppe, cheers, I'll try to get there soon | 08:12 |
| rogpeppe | fwereade: thanks | 08:12 |
| rogpeppe | fwereade: that last one gets us back to having unit and machine status for the gui folks | 08:13 |
| TheMue | rogpeppe: will look at them. | 08:20 |
| rogpeppe | TheMue: thanks! | 08:20 |
| TheMue | rogpeppe: a lot of stuff, but i'm getting forward | 09:24 |
| rogpeppe | TheMue: thanks a lot. the big changes are pretty much mechanical | 09:25 |
| TheMue | rogpeppe: yes, that helps | 09:26 |
| rogpeppe | TheMue: thanks for the review. i'm not sure what you're thinking of when you say "exposed constants for the EntityTypes" | 09:41 |
| TheMue | rogpeppe: simple public constants, nothing more. but as i also wrote, today they are needed nowhere, so simply forget it. ;) | 09:45 |
| rogpeppe | TheMue: sorry, do you mean the entity kinds? i don't see any occurrence of the word EntityType in the source. | 09:46 |
| TheMue | rogpeppe: oh, sorry, yes. where you return strings like "service" etc. | 09:47 |
| rogpeppe | TheMue: ah, i see. | 09:47 |
| rogpeppe | TheMue: they are actually compared, but not outside the params package. | 09:48 |
| TheMue | rogpeppe: i haven't seen them as strings elsewhere, only that you fill that one map by reflection. am i right? | 09:49 |
| rogpeppe | TheMue: look for the string "machine" in state/api/params and you'll see what i mean | 09:50 |
| danilos | heya, I am getting a bunch of test failures with juju-core trunk on raring: http://pastebin.ubuntu.com/5691888/ | 09:51 |
| TheMue | rogpeppe: ah, ic. so probably constants make sense, but not public ones. | 09:51 |
| danilos | I did follow README and CONTRIBUTING docs, except that I am using system-installed mongodb | 09:52 |
| rogpeppe | TheMue: i'm happy just using the strings. test coverage is sufficient that you won't be able to get it wrong. | 09:52 |
| danilos | (because it's 2.2.4 and 2.2.0 was referenced in the README) | 09:52 |
| danilos | anyone has any ideas what did I mess up? | 09:52 |
| rogpeppe | danilos: i think you do need the SSL-capable mongo | 09:52 |
| rogpeppe | danilos: it looks like you might be using GOPATH wrongly | 09:53 |
| rogpeppe | danilos: what's the value of your $GOPATH ? | 09:53 |
| danilos | rogpeppe, it's /home/danilo/.gopkgs | 09:53 |
| danilos | rogpeppe, I've got a symlink from ~/.gopkgs/src/launchpad.net/juju-core to ~/juju-core, but it behaves the same no matter what directory I am in | 09:54 |
| danilos | rogpeppe, or maybe not the same, but I've seen a lot of test failures either way | 09:54 |
| rogpeppe | danilos: symlinks don't work, i'm afraid | 09:55 |
| rogpeppe | danilos: your juju source must live in /home/danilo/.gopkgs/src/launchpad.net/juju-core | 09:55 |
| rogpeppe | danilos: you can symlink the other way if you want | 09:55 |
| danilos | rogpeppe, :/ but originally I tried it like that as well, maybe the non-SSL mongo was to blame | 09:55 |
| rogpeppe | danilos: you definitely need the SSL-capable mongo | 09:56 |
| rogpeppe | danilos: were you getting build errors before? | 09:56 |
| danilos | rogpeppe, no, go build seems to work correctly either way | 09:57 |
| rogpeppe | danilos: sorry, i'm mean build errors when testing | 09:57 |
| rogpeppe | danilos: like the errors you pasted above | 09:57 |
| rogpeppe | danilos: the easiest way to start is to do: "go get launchpad.net/juju-core/..." | 09:58 |
| rogpeppe | danilos: making sure you remove that symlink first | 09:58 |
| danilos | rogpeppe, right, that's how I started off then rearranged the trees a bit to follow my branch organization model (which is a shared repo with lightweight checkouts: I like having multiple branches around, especially when starting on a project) | 09:58 |
| rogpeppe | danilos: that will get you the source into the right place | 09:58 |
| mgz | go is really picky about placement of things | 09:59 |
| rogpeppe | danilos: i use cobzr | 09:59 |
| rogpeppe | danilos: fwereade doesn't but i don't know how he *does* use multiple branches | 09:59 |
| danilos | rogpeppe, right, I hate the git model of a single working directory and local merge problems when you forget to commit stuff, which is why I tried to avoid it | 09:59 |
| mgz | jam has a setup like yours, I'll just see if I have what his layout was anywhere | 09:59 |
| mgz | it certainly can be made to work | 09:59 |
| mgz | ...I think thumper does as well | 10:00 |
| rogpeppe | mgz: i think thumper uses native bzr colocation | 10:00 |
| * mgz has been using this to test the 2.6 colo stuff | 10:00 | |
| danilos | rogpeppe, it seems there are no build failures when I get the code directly in, argh :/ there're still a bunch of test failures | 10:01 |
| rogpeppe | danilos: are you using the right version of mongo? | 10:01 |
| danilos | mgz, rogpeppe: ok, I'll check with them if I don't figure something out, and switch to cobzr model until then | 10:02 |
| danilos | rogpeppe, not yet, I'll get it downloaded and set up as well | 10:02 |
| rogpeppe | danilos: all you need to do is download the tar file and put the files somewhere in your $PATH | 10:02 |
| danilos | rogpeppe, ok | 10:04 |
| danilos | rogpeppe, so far it seems that was the remaining problem; I'll figure out a tree layout that works for me, thanks for the input | 10:08 |
| rogpeppe | danilos: cool, np | 10:08 |
| mgz | I know one thing that works, is having the normal shared-repo+treeless branches in your normal dev location | 10:09 |
| mgz | then lightweight checkouts in the go-expected locations | 10:09 |
| mgz | there's a trick then to make switch easy, by doing something in locations.conf I think, but am not sure what it is | 10:10 |
| danilos | mgz, right, and can probably live with something like that, thanks | 10:15 |
| rogpeppe | fwereade: i'd really appreciate your comments on this branch before it gets waved through, as it's the technique that's directly impacted by the doc split between unit/machine and status. https://codereview.appspot.com/8534043/ | 10:19 |
| fwereade | rogpeppe, ok, looking now | 10:20 |
| rogpeppe | fwereade: thanks | 10:22 |
| fwereade | rogpeppe, that one's approved | 10:31 |
| fwereade | rogpeppe, need to grab some lunch, bbiab | 10:32 |
| rogpeppe | fwereade: thanks! | 10:32 |
| rogpeppe | fwereade: ping | 10:50 |
| fwereade | rogpeppe, pong | 11:08 |
| rogpeppe | fwereade: i was just wondering why we don't just use constraints.Value directly as a mongo doc | 11:09 |
| dimitern | mgz: standup? | 11:30 |
| mgz | I'm there | 11:31 |
| danilos | mgz, dimitern: https://pastebin.linaro.org/2129/ | 11:32 |
| danilos | mgz, dimitern: http://pastebin.ubuntu.com/5692068/ | 11:33 |
| dimitern | TheMue: cheers for updating the topic! | 11:37 |
| TheMue | dimitern: yep, seen it as my task as reviewer today. ;) | 11:38 |
| fwereade | rogpeppe, sorry, I left a comment half-types | 11:38 |
| fwereade | rogpeppe, because I'd rather have a dumb but explicit translation layer between the api type and the database representation than to conflate the two | 11:39 |
| dimitern | no critical bugs? how come? | 11:39 |
| rogpeppe | fwereade: do you think we should do that for all the other types we marshal directly to mongo then? | 11:39 |
| rogpeppe | fwereade: i'm wondering if we should marshal constraints to mongo as string | 11:40 |
| bac | anyone have time for a quick review? https://codereview.appspot.com/8532043 | 11:40 |
| fwereade | rogpeppe, I can't think of many of those... state.Tools, any others? | 11:40 |
| rogpeppe | fwereade: charm.Meta | 11:40 |
| rogpeppe | fwereade: (that's a big one) | 11:41 |
| fwereade | rogpeppe, hmm, yeah, in hindsight the pressures maybe feel different now constraints is out of state | 11:42 |
| rogpeppe | fwereade: charm.Config | 11:42 |
| rogpeppe | fwereade: i'd quite like a tool that automatically checked for mongo compatibility between versions | 11:43 |
| rogpeppe | fwereade: but i'm not entirely sure how it would work | 11:43 |
| fwereade | rogpeppe, yeah, I think it'd be tricky | 11:44 |
| rogpeppe | fwereade: there are two approaches: 1) make some kind of formal description of the schema and check that everything adheres to it 2) write a test that talks to two different versions, each of which looks at the same mongodb, getting one to write and the other to read | 11:46 |
| rogpeppe | fwereade: (at least) two approaches :-) | 11:46 |
| fwereade | rogpeppe, yeah, it's not going to be fun | 11:47 |
| rogpeppe | fwereade: maintaining compatibility is crucial though. | 11:48 |
| fwereade | rogpeppe, agreed | 11:48 |
| rogpeppe | fwereade: there's another possibility actually | 11:48 |
| fwereade | rogpeppe, I'm honestly not sure about external types, but I am starting to feel that from this perspective it's important that we keep a clear eye on just wtf we are storing in mongo | 11:49 |
| rogpeppe | fwereade: use a tool to tell us exactly what types we're storing in mongo | 11:49 |
| rogpeppe | fwereade: then we can use a variant of the go compatibility checker to make sure we don't change those types in backwardly incompatible ways. | 11:49 |
| rogpeppe | fwereade: it won't work for types that know how to marshal themselves, but those are probably not where the problems will occur | 11:50 |
| rogpeppe | fwereade: because if you change a MarshalBSON method, you'll have a pretty good idea that it might have some impact on mongo storage | 11:51 |
| fwereade | rogpeppe, well, it's the types that know how to marshal themselves I am concerned about tbh -- those changes are going to be easy to make accidentally | 11:51 |
| rogpeppe | fwereade: i'm more concerned about someone just changing the type of a field | 11:51 |
| rogpeppe | fwereade: without realising that the type is used in mongo | 11:52 |
| fwereade | rogpeppe, yep, that too | 11:52 |
| fwereade | rogpeppe, agreed that having what you described in place would take one big worry off the table | 11:52 |
| fwereade | rogpeppe, in the meantime I think I *would* prefer to avoid directly storing external types in mongo, but obviously we can't change all that now, we just need to fret and hope :/ | 11:53 |
| rogpeppe | fwereade: for now, perhaps we should just comment every type that's used in mongo, so that people know it's important not to change it. | 11:54 |
| rogpeppe | fwereade: it should be straightforward to do | 11:54 |
| fwereade | rogpeppe, that sounds like a good idea to me | 11:54 |
| rogpeppe | fwereade: i've added a ticket to the kanban | 11:59 |
| fwereade | rogpeppe, cheers | 11:59 |
| danilos | mgz, dimitern: the other test seems to really be fixed by r39 in goyaml from a week or so ago by gustavo | 12:11 |
| dimitern | rogpeppe, fwereade: next chapter in the nonced provisioning saga: https://codereview.appspot.com/8561044/ | 12:14 |
| mgz | trivial test fix branch to review: https://codereview.appspot.com/8570043 | 12:15 |
| mgz | danilos: ^you can practice your reitveld on that too :) | 12:15 |
| danilos | mgz, I will, that's even easier :) | 12:16 |
| danilos | I am having some trouble getting lbox to do anything with codereview.appspot.com: I suppose I need to configure something (even if I set my email address for bzr to the one I use to log in to codereview.appspot.com, no review is created [one is created on LP]) | 12:49 |
| rogpeppe | TheMue: here's a review for you: https://codereview.appspot.com/8568044 | 13:22 |
| danilos | ah, ok, so if updating the milestone fails (which I don't have permissions for yet), then it borks out and doesn't do a code review submit | 13:22 |
| TheMue | rogpeppe: just doing dimiters, then yours will follow | 13:22 |
| rogpeppe | TheMue: ta! | 13:23 |
| dimitern | danilos: how did you manage to update a milestone with lbox? :) | 13:23 |
| danilos | dimitern, I didn't, but it tries to update a bug milestone and I am still not a member of the ~juju team which is the maintainer for the juju-core project | 13:23 |
| danilos | dimitern, I am only passing the -bug=... parameter to the propose command | 13:24 |
| dimitern | danilos: I see | 13:24 |
| dimitern | danilos: didn't know it touches milestones when you link a bug | 13:24 |
| danilos | dimitern, yeah, it tries to create a bug if you don't link it (or so it seems) | 13:25 |
| danilos | now I need Kapil, Gustavo or Dave Cheney to add me to the team | 13:26 |
| fwereade | dimitern, reviewed | 13:26 |
| danilos | hazmat, hi, can you please add me to the ~juju team? | 13:26 |
| dimitern | fwereade: cheers! | 13:27 |
| dimitern | fwereade: and this is the final step: https://codereview.appspot.com/8561045/ | 13:28 |
| fwereade | dimitern, ta | 13:28 |
| hazmat | danilos, welcome to juju core, and done | 13:28 |
| danilos | hazmat, thanks! :) | 13:30 |
| dimitern | fwereade: I don't think asserting on both instanceid and nonce being "" will work for replayed transactions | 13:31 |
| danilos | TheMue, hi, I wouldn't mind getting a review for https://codereview.appspot.com/8572043/ :) | 13:32 |
| fwereade | dimitern, I think replayed transactions leak badly here anyway | 13:33 |
| fwereade | dimitern, but in the actual use cases, we'll never be setting the same instance id anyway, so that specific check actually reduces to "never set" anyway | 13:33 |
| TheMue | danilos: enqueued, just doing rogers ;) | 13:33 |
| danilos | TheMue, thanks | 13:33 |
| danilos | mgz, dimitern: I wouldn't mind if one of you gives me a second ack (or nack, depending on your opinion :) on https://codereview.appspot.com/8572043/ | 13:34 |
| fwereade | dimitern, I think it's clearer and simpler and no less correct ;p | 13:34 |
| dimitern | fwereade: it's simpler for sure, just have a nagging feeling I'm missing something, if i don't assert instid should either be "" or "instid" | 13:36 |
| dimitern | fwereade: perhaps you could alleviate my concern - what'll happen when we need to replay a SetProvisioned transaction without asserting "instid", which is already set? | 13:37 |
| fwereade | dimitern, I think there is a general issue wrt txns that need to be synchronized with non-txn state | 13:37 |
| fwereade | dimitern, the short answer is "we're probably screwed", but asserting on the possibility of the same instance id can't actually help, I think | 13:37 |
| dimitern | fwereade: istm asserting only on "" will cause erraborted, which then by the virtue of the machine being still alive will return "already set" | 13:38 |
| dimitern | fwereade: that's the only reason to check for same id in the assert, if you think it's superfluous, I'll drop it | 13:39 |
| dimitern | danilos: you've got a review | 13:41 |
| fwereade | dimitern, essentially a delayed SetProvisioned txn leaves state badly inconsistent with reality until it's completed, and there's no way round that except to do some distracting cleverness re completing machine txns before making provisioning-related decisions | 13:41 |
| fwereade | dimitern, and it's low-impact enough that I plan to just skip it for now | 13:42 |
| dimitern | fwereade: yeah, and hard to induce such an error to test it properly imo | 13:43 |
| dimitern | fwereade: ok, I'll do as you suggest | 13:43 |
| fwereade | dimitern, yeah, indeed | 13:43 |
| danilos | dimitern, thanks | 13:47 |
| danilos | dimitern, fwiw, I did test it with both tip revisions and going back one revision of goyaml to confirm it fails expectedly with it | 13:48 |
| dimitern | danilos: cheers | 13:54 |
| TheMue | rogpeppe, danilos: you've got reviews | 13:55 |
| rogpeppe | TheMue: ta! | 13:55 |
| TheMue | rogpeppe: yw, nice little helpers in the code | 13:56 |
| TheMue | danilos: does go get -u retrieves the fixed goyaml version? | 13:58 |
| danilos | TheMue, I can try reverting to r38 to see if it does (since I started yesterday, I got the latest version by default: r39 is from April 2nd) | 13:58 |
| danilos | TheMue, yeah, it does update it for me at least | 14:00 |
| TheMue | danilos: that's good | 14:01 |
| danilos | TheMue, btw, is "LGTM" a convention of rietveld (i.e. does it interpret it as a positive review and makes the review message green), or just internal juju team convention to give out a shorter "looks good to me"? | 14:01 |
| mgz | it's for the pretty green | 14:01 |
| danilos | mgz, for some definition of pretty, yes :) | 14:02 |
| * TheMue almost missed hangout | 14:02 | |
| TheMue | danilos: it is interpreted by rietveld, and "not lgtm" marks it red | 14:03 |
| dimitern | danilos: and there's the red NOT LGTM for Disapprove | 14:03 |
| danilos | TheMue, dimitern: ack, thanks | 14:03 |
| dimitern | danilos: not sure if it's case-sensitive, but it definitely catches it as part of the word (like "this is LGTMy"), ymmv | 14:05 |
| dimitern | fwereade: updated https://codereview.appspot.com/8561044/ | 14:07 |
| dimitern | fwereade: and there's the other one as well - https://codereview.appspot.com/8561045/ | 14:08 |
| * dimitern lunch | 14:09 | |
| mgz | heh, even later lunch than me | 14:10 |
| danilos | niemeyer, heya, I've marked a couple of bugs that you've fixed in goyaml with r39 as fix-committed, I hope you don't mind :) | 14:12 |
| niemeyer | danilos: Thanks! | 14:36 |
| niemeyer | danilos: They're even released already | 14:36 |
| danilos | niemeyer, even better :) | 14:39 |
| rogpeppe | danilos: are you Данило Шеган ? | 14:46 |
| rogpeppe | danilos: 'cos if so, i think you've just earned your first "broken trunk" badge :-) | 14:47 |
| TheMue | rogpeppe: with an updated goyaml? | 14:48 |
| rogpeppe | TheMue: yes | 14:48 |
| TheMue | rogpeppe: oh, where does it break? | 14:49 |
| rogpeppe | TheMue: constraints | 14:50 |
| TheMue | danilos: you've done go build ./... and go test ./... in the juju-core directory? | 14:50 |
| danilos | TheMue, yeah, with latest trunk | 14:50 |
| danilos | TheMue, are you seeing any problems? | 14:50 |
| danilos | rogpeppe, btw, I am, what did break for you? | 14:51 |
| danilos | rogpeppe, got the traceback? | 14:51 |
| mgz | ha, that would be nice :) | 14:51 |
| mgz | correct term is "cryptic one-line error" | 14:52 |
| rogpeppe | danilos: http://paste.ubuntu.com/5692522/ | 14:52 |
| danilos | rogpeppe, that's exactly what should fail if you don't have fixed goyaml version | 14:52 |
| rogpeppe | mgz: aww come on, it's a cryptic four-line error, surely?! | 14:52 |
| rogpeppe | danilos: ah, so i need to pull goyaml? | 14:53 |
| danilos | rogpeppe, i.e. when I tested it with r38 of goyaml, it failed like that | 14:53 |
| rogpeppe | danilos: ah, i'm on v36 | 14:53 |
| danilos | rogpeppe, yes, as per my email (this is goyaml r39 from 2013-04-02 by niemeyer): it was failing for me since I just fetched it yesterday for the first time | 14:53 |
| mgz | that probably deserves a post to the mailing list then, if everyone needs to update | 14:54 |
| mgz | ...which you've already done | 14:54 |
| danilos | mgz, I posted to juju-dev | 14:54 |
| rogpeppe | danilos: ahhhh, i think gustavo must've moved the goyaml repo | 14:54 |
| niemeyer | Hm? | 14:54 |
| rogpeppe | danilos: because it thinks 36 is the latest version | 14:54 |
| TheMue | rogpeppe: that's why i asked for the goyaml. *phew* | 14:54 |
| mgz | yeah, need `bzr pull --remember lp:goyaml` | 14:55 |
| danilos | rogpeppe, what does 'bzr info' say for you in there? | 14:55 |
| rogpeppe | danilos: http://paste.ubuntu.com/5692535/ | 14:55 |
| mgz | changed from lp:~gophers/goymal/trunk | 14:55 |
| danilos | it should be something like bzr+ssh://bazaar.launchpad.net/+branch/goyaml off the top of my head, let me check on disk for me | 14:55 |
| rogpeppe | niemeyer: i've done "bzr pull" in my goyaml directory and it things 36 is the latest branch, but apparently there's a r39 out there | 14:56 |
| mgz | rogpeppe: ^see my bzr pull command above | 14:56 |
| niemeyer | It's still lp:goyaml.. it's under ~goyaml's trunk now, indeed.. the change was announced when we switched everything out of ~gophers | 14:56 |
| danilos | rogpeppe, yeah, lp:goyaml points at http://bazaar.launchpad.net/~goyaml/goyaml/trunk/ for me (so not bzr+ssh, guess that's go thingy) | 14:56 |
| niemeyer | danilos: Nope | 14:57 |
| mgz | yeah, go get is... not helpful | 14:57 |
| niemeyer | Ah, yes | 14:57 |
| rogpeppe | niemeyer: ah, presumably i had to do a bzr pull --overwrite --remember lp:goyaml/trunk | 14:57 |
| niemeyer | rogpeppe: Hmm.. no.. | 14:57 |
| niemeyer | rogpeppe: bzr pull lp:goyaml --remember | 14:57 |
| danilos | niemeyer, any other reason why it didn't save the "top" URL of lp:goyaml (which I think evaluates to bzr+ssh://bazaar.launchpad.net/+branch/goyaml or similar) | 14:58 |
| niemeyer | danilos: I have no idea.. | 14:58 |
| rogpeppe | niemeyer: ah, ok, done, thanks | 14:58 |
| niemeyer | danilos: I'd expect that too | 14:58 |
| * rogpeppe wonders what other packages are "frozen" in the same way | 14:59 | |
| danilos | niemeyer, I guess it's not using "lp:" for fetching branches, which would explain it | 14:59 |
| mgz | yeah, jam proposed a fix for that | 14:59 |
| niemeyer | danilos: Maybe.. I'd not expect that to make a difference, though | 14:59 |
| niemeyer | danilos: Whatever is fetching branches is still talking about launchpad.net/goyaml | 14:59 |
| mgz | it may even have landed in go trunk | 15:00 |
| niemeyer | danilos: Not ~foo/goyaml/trunk | 15:00 |
| danilos | niemeyer, maybe it's doing so over http (I had the http ~foo/goyaml/trunk in it after a simple 'go get', and my bzr login is set properly) | 15:01 |
| niemeyer | danilos: Maybe, but I'd still consider that a bug in bzr itself | 15:01 |
| niemeyer | danilos: If the user asks for lp.net/foo, that's what it should be linked with | 15:01 |
| niemeyer | danilos: Even if it fetches from elsewhere | 15:02 |
| niemeyer | danilos: We probably won't solve that bug today, though | 15:02 |
| danilos | niemeyer, yeah, or in Launchpad (I've just double-checked that, if I do a 'bzr pull --remember http://launchpad.net/goyaml', it does the wrong thing) | 15:02 |
| dimitern | still waiting for a review on https://codereview.appspot.com/8561045/ | 15:02 |
| danilos | niemeyer, indeed :) | 15:02 |
| dimitern | fwereade: sorry, but i'm bugging you again ^^ | 15:02 |
| mgz | there does seem to be a launchpad forwarding bug | 15:02 |
| fwereade | dimitern, sorry dude | 15:03 |
| mgz | but... using that http url is... more than a little bogus anyway | 15:03 |
| niemeyer | mgz: Hmm.. why? | 15:04 |
| danilos | mgz, yeah, LP sends permanent redirects for that http url (it's not perfect, but it seems to work and I suppose go get uses it) | 15:04 |
| TheMue | dimitern: started revisioning | 15:04 |
| danilos | niemeyer, I don't think it's advertised anywhere as the URL to use for fetching branches | 15:04 |
| dimitern | TheMue: thanks | 15:04 |
| niemeyer | danilos: Well.. 1) It works; 2) It's extremely user-friendly | 15:04 |
| danilos | niemeyer, I am not disagreeing, I think there's a LP (or maybe bzr) bug in there | 15:06 |
| mgz | the branch also has a broken parent reference | 15:06 |
| rogpeppe | danilos: i can't see your post to juju-dev anywhere. what was the subject line? | 15:06 |
| niemeyer | rogpeppe: Update goyaml dependency for juju-core | 15:07 |
| rogpeppe | niemeyer: ah, just turned up | 15:07 |
| mgz | niemeyer: can you run this please: `bzr config --scope=branch -d bzr+ssh://bazaar.launchpad.net/+branch/goyaml --remove parent_location` | 15:08 |
| danilos | rogpeppe, I've posted an update with your experience as well, hope this helps somebody else who hits the same problem | 15:08 |
| rogpeppe | danilos: thanks | 15:09 |
| danilos | rogpeppe, and now I see that mgz beat me to it... oh well :) | 15:09 |
| mgz | : | 15:09 |
| mgz | ) | 15:09 |
| niemeyer | mgz: What does that do? | 15:11 |
| mgz | removes a broken parent location reference on the trunk branch | 15:12 |
| mgz | it's trunk, it shouldn't have a parent (and certainly not a ../../ one that goes nowhere) | 15:12 |
| TheMue | dimitern: you've got a review | 15:12 |
| dimitern | TheMue: tyvm | 15:12 |
| niemeyer | mgz: Never seen that | 15:12 |
| niemeyer | mgz: Neat | 15:13 |
| niemeyer | mgz: The command, Imean | 15:13 |
| mgz | beats editing .bzr/branch/branch.conf, especially when it's a remote branch :) | 15:14 |
| danilos | phew | 15:15 |
| * danilos hands the first "broken trunk" badge back ;) | 15:15 | |
| mgz | okay, I think that probably does fix the launchpad redirect too | 15:16 |
| mgz | so, go get should now have the right origin | 15:16 |
| niemeyer | mgz: Done | 15:16 |
| danilos | mgz, it does? I just filed https://bugs.launchpad.net/launchpad/+bug/1166854, let me check that :) | 15:23 |
| _mup_ | Bug #1166854: bzr http URL stores evaluated URL instead of the link <Launchpad itself:New> < https://launchpad.net/bugs/1166854 > | 15:23 |
| danilos | mgz, it doesn't for me, it still stores the evaluated URL instead of the shorter one | 15:23 |
| mgz | oh, that bxr resolves redirects and stores the dest is a known thing | 15:24 |
| danilos | mgz, at least 'bzr pull --remember http://launchpad.net/goyaml' does (haven't tried 'go get' itself) | 15:24 |
| mgz | but launchpad actually had the wrong redirect | 15:24 |
| danilos | mgz, right; oh, the redirect worked fine for me at least | 15:24 |
| danilos | mgz, btw, is bzr supposed to store temporary redirects as well? could that be a solution? | 15:25 |
| danilos | mgz, solution on the LP side, that is | 15:25 |
| mgz | the launchpad side for this is very messy | 15:25 |
| mgz | there are two levels, the first of which isn't actually an http redirect at all, but a fake branch that references another location | 15:25 |
| danilos | mgz, it's starting to sound nice | 15:26 |
| danilos | mgz, "interesting" is a perfect word, I assume :) | 15:26 |
| danilos | anyway, enough of that for now, those smarter about code-hosting in LP and bzr can comment on the bug and close it as invalid if needed; it'd still be nice to improve the experience for potential Go Launchpad users | 15:28 |
| dimitern | niemeyer: reviewed | 15:29 |
| niemeyer | dimitern: Cheers | 15:29 |
| mgz | sorry, http->https redirect first... then: | 15:29 |
| mgz | $ curl https://launchpad.net/goyaml/.bzr/branch/location | 15:29 |
| mgz | http://bazaar.launchpad.net/~goyaml/goyaml/trunk | 15:29 |
| niemeyer | fwereade: Any idea of when we'll make log messages reasonable? | 15:38 |
| danilos | I am seeing a problem in environs/openstack/config_test.py configTest.check method: it doesn't seem to clean the state properly after a failing test: I expect first test to fail (I am just adding it before adding the code), but the second one shouldn't fail: if I move my new test to the end, second one doesn't fail anymore | 15:39 |
| fwereade | niemeyer, ...damn, I have a draft email proposing a small doable card | 15:39 |
| niemeyer | fwereade: Just got a request by dimitern to uglify messages to conform to other ugly messages | 15:39 |
| niemeyer | fwereade: I'm doing it, but the output is comical | 15:39 |
| niemeyer | 2013/04/09 12:39:56 INFO JUJU:juju:publish cmd/juju: charm published at 2013-04-01T15:53:05Z as cs:~niemeyer/precise/ubuntu-6 | 15:40 |
| fwereade | danilos, that sounds like a problem to me | 15:40 |
| dimitern | niemeyer: i think a consistent source is better than half-baked better solution only at places | 15:40 |
| niemeyer | JUJU! JUJU! JUJU! I mean it, JUJU! | 15:40 |
| niemeyer | dimitern: I'm doing it.. | 15:40 |
| dimitern | niemeyer: and i agree the format needs to change to cut the bullshit of the logs :) | 15:40 |
| niemeyer | dimitern: But I don't have to agree.. :) | 15:40 |
| fwereade | niemeyer, the proposal will eliminate that at least -- thank you for bearing with us re consistency in the meantime, though | 15:40 |
| danilos | fwereade, http://pastebin.ubuntu.com/5692650/ and my test is in lp:~danilo/juju-core/bug-1135335 | 15:41 |
| niemeyer | fwereade: np.. I'm more bothered by presenting people with such obviously bad UX than the fact I have to change it | 15:42 |
| dimitern | fwereade: i though the JUJU: badge was supposed to somehow help the logs integrate better with syslog, but it's kinda crap if it's a requirement - a smart syslog will badge these according to sources | 15:43 |
| fwereade | danilos, omg, that's evil | 15:43 |
| fwereade | danilos, var credentialsTestConfig = configTests[12] | 15:43 |
| danilos | fwereade, aah | 15:44 |
| fwereade | danilos, I haven't checked your branch, but I bet you inserted a test before that one, thus throwing off the hardcoded magiv number | 15:44 |
| mgz | yeah, that is good. | 15:44 |
| fwereade | danilos, please kill that technique with fire if you would be so good :) | 15:44 |
| danilos | fwereade, I'd be delighted to :) any suggestions as to how since I am just getting to grips with go syntax? :) | 15:45 |
| danilos | fwereade, should I simply split it out into a separate var, and then reference it in the big array after that? | 15:45 |
| mgz | move it above the block of test definitions as its own thing, then add it in the list | 15:45 |
| fwereade | danilos, easiest way: just stick that test in a var, then stick the var into the []tests | 15:45 |
| fwereade | danilos, but cast an eye over the actual uses of it | 15:45 |
| danilos | mgz, fwereade: ok, I guess we are all on the same page, thanks :) | 15:45 |
| danilos | fwereade, of course | 15:45 |
| fwereade | danilos, it may be that it's not worth it in the first place ;) | 15:46 |
| danilos | fwereade, right, it's used in a single place with a different test.err and a couple of different env vars (just below the declarations): I'll see if I can move that as a separate entry in the configTests array | 15:48 |
| dimitern | today i tried the classic bootstrap+deploy mysql/wordpres+add-unit for both+expose; in ec2 - everything worked with the new provisioning! | 15:48 |
| fwereade | danilos, great, thanks | 15:48 |
| * fwereade cheers at dimitern | 15:49 | |
| * danilos tries to count from 0 to 12 in an array :) | 15:49 | |
| fwereade | dimitern, https://codereview.appspot.com/8561044/ LGTM | 15:49 |
| fwereade | dimitern, btw, what's the value of BootstrapNonce? | 15:50 |
| fwereade | dimitern, didn't spot that | 15:50 |
| dimitern | fwereade: "user-admin:bootstrap", as you suggested - it was part of the previous CL | 15:50 |
| fwereade | dimitern, ah, great, thanks | 15:50 |
| dimitern | fwereade: thanks for the review; i really want to finish and land both of these today | 15:51 |
| fwereade | dimitern, what was the thinking behind UUID for nonce? | 15:52 |
| fwereade | dimitern, seems a shame not to have it badged with the provisioning machine | 15:53 |
| fwereade | dimitern, nothing wrong with a UUID in itself, but I'd prefer the nonce to look like "machine-0:<UUID>" if we're going with that rather than a sequence | 15:54 |
| niemeyer | publish is going in! | 15:54 |
| * fwereade cheers at niemeyer | 15:55 | |
| niemeyer | and the server has been updated last night, which means it should work for everybody using tip already | 15:55 |
| dimitern | fwereade: fair enough, machine-0:<uuid> it is | 16:06 |
| * rogpeppe eats the last piece of Easter chocolate | 16:06 | |
| TheMue | rogpeppe: enjoy, i'll have dinner in a few moments. today no lunch. :/ | 16:07 |
| dimitern | i need two LGTMs still on the following CLs: https://codereview.appspot.com/8561044/ and https://codereview.appspot.com/8561045/ | 16:36 |
| dimitern | fwereade: ping for the second one | 16:38 |
| rogpeppe | dimitern: looking | 16:39 |
| rogpeppe | dimitern: i know that we have to set the nonce at the same time as the instance id, so SetProvisioned is reasonable there (i'm not keen on the name, but can't think of a better one currently), but i think InstanceId should just return the instance id, and a new method InstanceNonce (?) can return the nonce. no need to make calling instanceid harder, i think. | 16:41 |
| rogpeppe | dimitern: it's a pity that SetInstanceId is used as a test example everywhere! | 16:42 |
| dimitern | rogpeppe: not keen on having a way to get the nonce directly, it's supposed to be internal | 16:43 |
| rogpeppe | dimitern: huh? it's blatantly not internal - the function returns it! | 16:43 |
| dimitern | rogpeppe: I mean it's intentional not having a way to get it, you're supposed to check if it's sane with CheckProvisioned only | 16:44 |
| rogpeppe | dimitern: another possibility would be to define InstanceId as struct {Id string; Nonce string}, i suppose | 16:44 |
| rogpeppe | dimitern: in which case, don't return it from InstanceId | 16:44 |
| dimitern | rogpeppe: InstanceId returning a bool is kinda half-useful now, because we have CheckProvisioned, i agree | 16:45 |
| dimitern | rogpeppe: it's not returned from InstanceId | 16:45 |
| rogpeppe | dimitern: ah, sorry, i misread | 16:46 |
| dimitern | rogpeppe: so InstanceId() -> InstanceId and that's it? | 16:47 |
| rogpeppe | dimitern: i'm not sure i understand you there | 16:48 |
| dimitern | rogpeppe: if i understood you correctly, you're suggesting to change machine.InstanceId() to return InstanceId only, rather than InstanceId and bool (instance != "")? | 16:49 |
| rogpeppe | dimitern: no, not really. i lost that argument aeons ago. | 16:49 |
| rogpeppe | dimitern: i'd misinterpreted a change in your CL | 16:49 |
| dimitern | rogpeppe: we're living in a brave new world now, times for such changes, if they make sense | 16:50 |
| dimitern | :) | 16:50 |
| rogpeppe | dimitern: i'll leave it for now | 16:50 |
| dimitern | rogpeppe: works for me | 16:50 |
| rogpeppe | dimitern: hmm, i'm surprised tests pass. i thought there were probably various tests that assumed you could call SetInstanceId more than once | 16:56 |
| dimitern | rogpeppe: there was one in api_test, which I changed (TestMachineRefresh), to work like the similar test in state | 16:57 |
| rogpeppe | dimitern: ah, i wondered why TestMachineRefresh had changed so much | 16:58 |
| dimitern | rogpeppe: and there was another one calling SetInstanceId(x, y) and then SetInstanceId("", ""), where I removed the second case, because the next case is SetAgentTools(), which should also trigger a change | 16:58 |
| === deryck_ is now known as deryck[lunch] | ||
| danilos | TheMue: another one from me if you've got the time: https://codereview.appspot.com/8584043/ | 17:23 |
| danilos | fwereade, I suppose you might want to comment on the general approach at least: https://codereview.appspot.com/8584043/ | 17:24 |
| dimitern | danilos: I think he signed off already, I can take a look | 17:24 |
| danilos | dimitern, sure, I'd appreciate it (this is just a refactor to fix problems I hit in the test when adding my own code) | 17:25 |
| niemeyer | Trivial one: https://codereview.appspot.com/8585043 | 17:27 |
| dimitern | danilos: good change, but while you're at it, why not fix the source of the problem (ec2 provider, where this code was copied from)? | 17:30 |
| mgz | at least let him do that in a seperate mp :) | 17:30 |
| danilos | dimitern, heh, I did not know it was copied from there :) | 17:30 |
| danilos | dimitern, but yeah, let's do that in a separate MP, I want to get some traction on the actual bug fix as well | 17:31 |
| dimitern | mgz: well, it's all the same - the changes should be identical, and a bit less shit overall :) so win-win | 17:31 |
| dimitern | danilos: at least put a TODO(danilos) Fix this test to save/restore env vars, like openstack config_test ? | 17:32 |
| danilos | dimitern, sure | 17:32 |
| danilos | dimitern, I can try seeing if it's simple enough (depending on how similar they are: there might be some intricate test details that I don't want to just do over in haste) | 17:33 |
| dimitern | danilos: they should be almost identical, the check() func for sure | 17:34 |
| dimitern | danilos: but a TODO is fine, lest we forget about it | 17:34 |
| danilos | dimitern, a bunch of variables are renamed (looking at the diff in trunk), even in the configTest struct | 17:36 |
| dimitern | danilos: ok then, then it's better to leave it off for now | 17:36 |
| dimitern | niemeyer: reviewed | 17:43 |
| dimitern | danilos: you too | 17:43 |
| danilos | dimitern, thanks | 17:43 |
| niemeyer | dimitern: Danke! | 17:43 |
| danilos | dimitern, for the "invalid-mode", do you want me to define a const in the test (since this is a non-existant auth mode, the value is typed in as a throw-away string)? | 17:46 |
| dimitern | danilos: you can use AuthMode("invalid-mode") w/o problems, it'll be obvious even more I think | 17:47 |
| danilos | dimitern, ah, right, thanks | 17:48 |
| dimitern | danilos: or maybe even without the cast - since literal string consts in go are casted automatically | 17:48 |
| danilos | dimitern, it is being used without the cast right now, so now I am not sure what do you mean? | 17:48 |
| dimitern | danilos: when in doubt if anything will work, try it on https://play.golang.org/ - you can even share it :) | 17:49 |
| danilos | dimitern, you mean define it as a const in the test file and then use it there? | 17:49 |
| dimitern | oops - it's http: only, no s | 17:49 |
| danilos | dimitern, cool, nice tip, I'll see if it can replace what python shell does for me in python | 17:49 |
| dimitern | danilos: I mean try authMode: "invalid-mode", when authMode is typed, rather than just string - if it compiles, you're good, if not, cast it | 17:50 |
| danilos | dimitern, ah, ok | 17:50 |
| dimitern | danilos: the best thing about go is most things are compile-time checked | 17:51 |
| niemeyer | dimitern: If I can explore you one last time today: https://codereview.appspot.com/8589043 | 17:52 |
| niemeyer | dimitern: Also tiny | 17:53 |
| rogpeppe | eod for me. g'night all. | 17:53 |
| niemeyer | rogpeppe: Cheers! | 17:53 |
| dimitern | niemeyer: sure, np | 17:53 |
| rogpeppe | niemeyer: nnight | 17:53 |
| dimitern | rogpeppe: night! | 17:53 |
| niemeyer | I believe this is the last piece I'll be doing on publish right now | 17:54 |
| dimitern | niemeyer: LGTM | 17:55 |
| niemeyer | dimitern: Super, thanks! | 17:55 |
| niemeyer | fwereade: Are you around by any chance? | 17:56 |
| danilos | dimitern, it compiles, but it ends up being AuthMode for the expected value, and string for the received value: I suppose config.go makes it as string | 17:58 |
| dimitern | danilos: ah, bugger.. ok so reluctantly I'd go with strings then (why we have consts we're not using them also in tests? because the config stuff is crap basically..) | 18:00 |
| danilos | dimitern, right, but let's not fix all the problems in there today :) | 18:01 |
| dimitern | danilos: indeed :) | 18:01 |
| dimitern | 'night guys, i'm off | 18:02 |
| danilos | dimitern, night | 18:03 |
| niemeyer | danilos: Wanna do a second review on those two CLs? They're both tiny and easy going: https://codereview.appspot.com/8585043/, https://codereview.appspot.com/8589043 | 18:04 |
| niemeyer | Wow.. look at those numbers, btw | 18:04 |
| danilos | niemeyer, yeah, very confusing | 18:05 |
| danilos | niemeyer, I am not sure I know how to use rietveld to review stuff already, but sure (if you'd trust a 2nd day Go expert like myself :) | 18:06 |
| niemeyer | danilos: It's already the second review.. (or 3rd, in the case of the first change, as I talked to William about it ahead of time) | 18:06 |
| niemeyer | danilos: So, sure, I'm happy to take your review on it | 18:06 |
| niemeyer | danilos: Look at the CLs, if you have any comments/questions, double-click on the respective line and comment | 18:07 |
| niemeyer | danilos: When you're done with the whole review, click on "Publish+Mail comments" at the top | 18:07 |
| niemeyer | danilos: Include "LGTM" if you're happy with the changes to go in, or any other commnets | 18:07 |
| danilos | niemeyer, cool, I'll do them and you can be my second reviewer on https://codereview.appspot.com/8584043/ :) | 18:07 |
| === deryck[lunch] is now known as deryck | ||
| danilos | niemeyer, LGTM on both, one minor comment on the first one | 18:16 |
| niemeyer | danilos: LGTM as well, with a comment on the restoring | 18:18 |
| niemeyer | danilos: Sensible comments, thakns. | 18:19 |
| danilos | niemeyer, nice, thanks for reminding me of the defer | 18:20 |
| niemeyer | danilos: Yeah, it's a nice superpower we have in Go :) | 18:21 |
| niemeyer | danilos: Note that it's not just convenience.. it solves an actual issue because there are possible paths before the current restoring that could exit the function without restoring | 18:21 |
| danilos | niemeyer, right, I can imagine other errors being caught and handled in the code in between | 18:22 |
| * danilos -> off | 18:39 | |
| fwereade | niemeyer, hey, I'm back unless laura wakes back up | 19:50 |
| niemeyer | fwereade: Heya | 19:53 |
| niemeyer | fwereade: All good | 19:53 |
| niemeyer | fwereade: Already got reviews, and changes are in | 19:53 |
| niemeyer | fwereade: Including the validator changes we talked about | 19:53 |
| fwereade | niemeyer, you rock | 19:54 |
| fwereade | niemeyer, thank you very much :) | 19:54 |
| === gary_poster is now known as gary_poster|away | ||
| === gary_poster|away is now known as gary_poster | ||
| niemeyer | fwereade: It's been my pleasure | 20:46 |
| niemeyer | fwereade: Thanks for bearing with me as well :) | 20:46 |
| === markramm is now known as mramm_ | ||
| thumper | morning | 21:13 |
| * thumper frowns | 22:14 | |
| thumper | fwereade: so... is there a go debugger that would allow me to step through a test? | 22:19 |
| thumper | to figure out what isn't working? | 22:19 |
| thumper | test fails when I'd expect it to pass | 22:19 |
| fwereade | thumper, hmm, I am not a debugger person, so all I can do is try to hunt down the blog post I've seen people mentioning | 22:20 |
| fwereade | thumper, but if the symptoms are amenable to concise description they might ring a bell for me | 22:20 |
| fwereade | thumper, http://golang.org/doc/gdb | 22:21 |
| thumper | fwereade: well, we could chat about it, hangout would be faster | 22:21 |
| fwereade | thumper, sure | 22:22 |
| thumper | fwereade: I'll start it | 22:23 |
| thumper | ? | 22:23 |
| fwereade | thumper, yes please, sorry | 22:23 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!