| bigjools | apps should not be rotating their own logs | 00:00 |
|---|---|---|
| bigjools | twisted tried that and it was miserably shit | 00:00 |
| davecheney | bigjools: was talking about reopening on HUP | 00:00 |
| thumper | wallyworld_: aren't you off today? | 00:57 |
| wallyworld_ | thumper: yeah, about to go to in | 00:57 |
| axw | thumper: if someone "not lgtm"s something, I need that same person to lgtm to go ahead - is that right? | 01:00 |
| thumper | axw: it depends :) | 01:01 |
| thumper | axw: what is it? | 01:01 |
| axw | just a sec | 01:01 |
| axw | thumper: https://codereview.appspot.com/12924043/ | 01:01 |
| * thumper looks | 01:01 | |
| davecheney | axw: depends, could you take them in a straight fight ? | 01:02 |
| axw | heh :) | 01:02 |
| thumper | axw: if you can address my comments, I can give you an LGTM | 01:11 |
| thumper | axw: william's opposition was due to no test, since you've added one, I think you'll be fine | 01:11 |
| * thumper goes to check on the pizza in the oven | 01:11 | |
| axw | thumper: thanks | 01:12 |
| axw | thumper: that LC_ALL thing is existing. do you want me to update those things as I go? | 01:16 |
| thumper | :) do you feel like making it more up to date? | 01:17 |
| axw | ... yes | 01:17 |
| axw | :) | 01:17 |
| axw | I think I'll write a tool to check import groups | 01:22 |
| axw | prevent myself from submitting crap | 01:22 |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1212903 | 01:26 |
| _mup_ | Bug #1212903: cmd/juju: debug hooks should set a better PS1 <papercut> <juju-core:New> <https://launchpad.net/bugs/1212903> | 01:26 |
| thumper | axw: That bug ^^^ seems pretty trivial to do right? | 01:34 |
| * axw looks | 01:34 | |
| axw | thumper: yes should be pretty simple | 01:35 |
| axw | I'll take a look in a moment | 01:37 |
| davecheney | thumper: axw yeah | 01:39 |
| davecheney | it would be super useful to have for our upcoming engagements | 01:39 |
| davecheney | we're always zooming the screen | 01:39 |
| davecheney | then the tmux output and the path become a real issue | 01:39 |
| davecheney | for a charm the standard path consumes almost all the screen width | 01:40 |
| thumper | davecheney: I fully agree with the bug description | 01:40 |
| thumper | I think it is sound | 01:41 |
| davecheney | sweet | 01:41 |
| davecheney | i didn't think it would be a technical issue to fix | 01:41 |
| davecheney | more ... philosophical | 01:41 |
| thumper | axw: you aren't looking at breaking up the agent config are you? | 02:13 |
| thumper | axw: because if you aren't, I'll take that on | 02:13 |
| axw | thumper: not right now | 02:13 |
| axw | okey dokey | 02:13 |
| thumper | I kinda need it for some other work | 02:13 |
| axw | thumper: nps. I'll work on bugs and continue with the manual provisioner | 02:14 |
| thumper | as after talking with william, he convinced me that shoving env vars into the upstart scripts isn't very good | 02:14 |
| axw | heh ok :) | 02:14 |
| thumper | rewriting upstart scripts on upgrade is a little crazy | 02:14 |
| thumper | but adding stuff to agent config isn't too bad | 02:14 |
| axw | right | 02:14 |
| * thumper recalls the need to write an email about juju containers | 02:16 | |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1212916 | 02:48 |
| _mup_ | Bug #1212916: uniter/worker: unit agent MUST remove stale unix socket <papercut> <juju-core:New> <https://launchpad.net/bugs/1212916> | 02:48 |
| thumper | davecheney: if you put an importance on the bug, please mark it triaged | 02:56 |
| davecheney | thumper: will do | 02:57 |
| davecheney | sorry | 02:57 |
| davecheney | will fix | 02:57 |
| thumper | davecheney: I've done those two | 02:57 |
| thumper | but for future reference | 02:57 |
| davecheney | ok, i understand now | 02:57 |
| axw | thumper: thanks, I've updated checkers -> jc. will merge | 03:00 |
| thumper | axw: ack | 03:33 |
| thumper | axw: if you run the trunk tests, do your ec2 ones pass? | 03:50 |
| * thumper wonders if goamz has been updated | 03:50 | |
| axw | thumper: goamz has indeed been updated | 03:50 |
| axw | are you getting a TestInstanceState failure? | 03:50 |
| thumper | yes | 03:51 |
| * thumper pulls goamz and tries again | 03:51 | |
| axw | I'll send an email now. Sorry, should have done it last week | 03:51 |
| thumper | I am prepared to buy drinks for whoever makes the ec2 tests run faster (without skipping or producing incorrect results) | 03:52 |
| axw | thumper: rogpeppe said he sped it down to 6s or something like that | 03:52 |
| thumper | but decided not to commit it? | 03:52 |
| axw | IIRC it required modifying goamz itself | 03:52 |
| axw | I think there's some global variable that would affect the normal operation | 03:52 |
| thumper | well, if he fixes it, I'll buy him some drinks :) | 03:52 |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1212936 | 04:36 |
| _mup_ | Bug #1212936: cmd/juju: debug-hooks without hook fired should place the user into a simple hook environment <papercut> <juju-core:Triaged> <https://launchpad.net/bugs/1212936> | 04:36 |
| davecheney | relation question | 05:39 |
| davecheney | can you run the relation-get hook command during the relation-departed hook ? | 05:39 |
| rogpeppe | mornin' all | 06:12 |
| axw | morning rogpeppe | 06:22 |
| rogpeppe | axw: hiya | 06:22 |
| === tasdomas_afk is now known as tasdomas | ||
| noodles775 | dimitern: Hi! If you have a few minutes at some point, can you read what I tried RE testing with RegisterControlPoint etc on the decscription of https://codereview.appspot.com/12795045/ , and let me know what I have missed, or whether it's something that needs to be added to goose? | 07:24 |
| dimitern | noodles775: I think an update to goose is in order, so that you can tweak the instance state in the test double | 07:37 |
| dimitern | noodles775: sorry, I was wrong that you can tweak it without code changes | 07:37 |
| noodles775 | dimitern: k - do you think I should do that before landing that branch (ie. so it doesn't land without tests), or it's OK as is? | 07:40 |
| === racedo` is now known as racedo | ||
| dimitern | noodles775: I don't think increasing the timeout fixes anything | 07:42 |
| dimitern | noodles775: it's still fragile | 07:42 |
| dimitern | noodles775: if anything, we should fix the nova server filter to be smarter and return stuff not only in ACTIVE or BUILD states, but also hp-cloud specific BUILD(spawning) | 07:43 |
| noodles775 | dimitern: Yeah, I did try that, but it's still fragile (takes about 8-9 seconds to get to BUILD(spawning) from my testing) | 07:44 |
| dimitern | noodles775: and to test it, you'll need to change goose to allow you to change the instance state when you start a server with the test double | 07:44 |
| dimitern | noodles775: ok, let me rephrase - timeout by itself is not enough to fix it, you'll need the above things as well | 07:45 |
| dimitern | noodles775: fixing the filter to use HasPrefix instead of == should be trivial | 07:47 |
| noodles775 | dimitern: That doesn't work - I tried that (see the description). | 07:47 |
| dimitern | noodles775: it doesn't work, because it returns BUILD(networking) and at that state the instance is not yet accessible? | 07:48 |
| noodles775 | dimitern: It means that it returns as soon as HP sets BUILD(networking), but juju-core then never connects (I didn't dig deeper, but I assume we make some assumptions)... | 07:48 |
| noodles775 | Yes. | 07:48 |
| dimitern | well, it seems obvious that the network is not yet up at that point | 07:48 |
| noodles775 | Right - but it continues to not connect. Let me paste you output, it may make more sense. | 07:49 |
| === tasdomas is now known as tasdomas_afk | ||
| === tasdomas_afk is now known as tasdomas | ||
| noodles775 | dimitern: http://paste.ubuntu.com/5991914/ | 07:58 |
| dimitern | noodles775: my point is, instead of HasPrefix, you could use server.Status == nova.StatusBuild || serverStatus == "BUILD(spawning)" (perhaps even define this as const HPStatusBuildSpawning) | 08:00 |
| dimitern | noodles775: then run it 10 times or more to see if it passes every time, if sometimes it doesn't, then increase the timeout | 08:01 |
| noodles775 | dimitern: Right, I did do that and it then takes about 8-9 seconds - so it is better than 15s that it takes for ACTIVE. If you think it's better to use that way, I'll do that. | 08:01 |
| dimitern | noodles775: the hp-specific build const should be definied in goose, like other nova.Status* consts I think, and a change to enable setting it in the double is in order, so you can test it | 08:02 |
| noodles775 | dimitern: yep, will do. | 08:03 |
| dimitern | noodles775: thanks! | 08:03 |
| noodles775 | dimitern: np, thanks for the feedback and direction. | 08:03 |
| dimitern | rogpeppe: hey | 08:03 |
| rogpeppe | dimitern: hiya | 08:03 |
| dimitern | noodles775: no worries, glad I can help | 08:03 |
| dimitern | rogpeppe: updated https://codereview.appspot.com/12990043/ | 08:03 |
| rogpeppe | dimitern: looking | 08:03 |
| dimitern | rogpeppe: cheers | 08:03 |
| rogpeppe | dimitern: reviewed | 08:13 |
| dimitern | rogpeppe: thanks | 08:15 |
| dimitern | fwereade: ping | 08:15 |
| dimitern | blast.. william is out today | 08:17 |
| dimitern | rogpeppe: I have a question | 08:17 |
| * rogpeppe is all ears | 08:17 | |
| dimitern | rogpeppe: now that I changed Relation() to return only the endpoint of the auth'ed unit, should it return []params.Endpoint at all or just a single one? | 08:17 |
| rogpeppe | dimitern: hmm, since there can only be one local endpoint, i think it should just return a single one | 08:18 |
| dimitern | rogpeppe: then I shouldn't really use params.RelationInfo, but a copy of it without the Endpoints slice, just a single field | 08:18 |
| rogpeppe | dimitern: i suppose so, but i can't get too worried about it. | 08:21 |
| rogpeppe | dimitern: the comment in UniterAPI.Relation should definitely be updated though - i missed that | 08:21 |
| dimitern | rogpeppe: ok | 08:21 |
| dimitern | rogpeppe: did you have time to check the charm store for possible relation names? | 08:23 |
| rogpeppe | dimitern: no, i'm going to do that this morning | 08:24 |
| dimitern | rogpeppe: great | 08:24 |
| rogpeppe | dimitern: i've also been trying to make some sort of sense of the config mess this morning. this is as far as i've got: https://docs.google.com/a/canonical.com/spreadsheet/ccc?key=0AghcHASXWzXmdHdOX2htQVJITkdaYVh4VnZ2Z0RrQWc#gid=0 | 08:25 |
| dimitern | rogpeppe: looking | 08:25 |
| rvba | Anyone up for a tiny review? https://codereview.appspot.com/12850045/ | 08:25 |
| rogpeppe | rvba: looking | 08:25 |
| rogpeppe | rvba: LGTM | 08:27 |
| rvba | rogpeppe: ta | 08:27 |
| dimitern | rogpeppe: for openstack, the username, password and tenant-name are not generated, only the password is secret; auth-url and auth-mode are cloud-specific and not generated, access-key and secret-key are used with auth-keypair only (otherwise username/password are used) - both secret, region is cloud-specific, not generated or secret, both buckets can be generated and are not secret | 08:34 |
| rogpeppe | dimitern: feel free to change the spreadsheet! | 08:35 |
| dimitern | rogpeppe: ah, I wasn't sure I can | 08:35 |
| rogpeppe | dimitern: if you're logged into your canonical account, you should be able to | 08:35 |
| dimitern | rogpeppe: yeah, I'm already half way through | 08:42 |
| rogpeppe | dimitern: yeah, i saw that - very helpful, thanks! | 08:42 |
| rogpeppe | rvba: if you have a moment at some point, could you perhaps fill out some of the azure-specific entries in this spreadsheet? https://docs.google.com/a/canonical.com/spreadsheet/ccc?key=0AghcHASXWzXmdHdOX2htQVJITkdaYVh4VnZ2Z0RrQWc#gid=0 | 08:44 |
| rogpeppe | rvba: i'm trying to start rationalising our config attribute twistiness | 08:44 |
| rvba | rogpeppe: sure, I'll have a look. | 08:46 |
| rogpeppe | does anyone know how to use the new charm browser to list all available charms? | 08:51 |
| rogpeppe | the old one was easy enough to screen scrape, but this one isn't so easy. | 08:51 |
| dimitern | rogpeppe: not sure about the last 4 columns though | 08:51 |
| rogpeppe | dimitern: "needed for sync tools" is whether an attribute is needed when pushing to the private storage | 08:52 |
| rogpeppe | dimitern: "needed for bootstrap" is whether the attribute needs to be passed into the bootstrap node | 08:53 |
| rogpeppe | dimitern: "needed for client operations after bootstrap" is whether you need to use the attribute when connecting to the state later. | 08:53 |
| dimitern | rogpeppe: I have to think about all of these - all in all they seem to be the same as for ec2, but have to check | 08:54 |
| rogpeppe | dimitern: i would expect them to be similar | 08:54 |
| === tasdomas is now known as tasdomas_afk | ||
| === tasdomas_afk is now known as tasdomas | ||
| rvba | rogpeppe: what does "OK to specify in environments.yaml?" mean exactly? | 09:19 |
| rogpeppe | rvba: do you want a user to be able to specify that attribute in the environments.yaml file? | 09:19 |
| rogpeppe | rvba: in your case, the answer is likely to be yes for all | 09:19 |
| rvba | Okay. | 09:20 |
| rogpeppe | rvba: we have some weird attributes like agent-version, which don't really make sense for a user to be able to specify there | 09:20 |
| rogpeppe | rvba: that's part of the twistiness | 09:20 |
| rvba | rogpeppe: I see. | 09:20 |
| rvba | rogpeppe: I'm a bit confused by "Cloud account information?". I interpret this as "The value part of one's cloud account information" but I see ec2's 'region' is checked (with a question mark though). | 09:36 |
| rogpeppe | rvba: it's whether the attribute should be considered part of the cloud account information (we're planning to move towards specifying cloud accounts as a separate entry in the juju conf file) | 09:37 |
| rogpeppe | rvba: just leave it blank if you're not sure | 09:37 |
| rvba | rogpeppe: 'region' does not seem to be part of the cloud account information then… it's more linked to the deployment isn't it? | 09:38 |
| rogpeppe | rvba: yes, you're probably right. | 09:39 |
| rogpeppe | rvba: i'm not quite sure of fwereade's conception of accounts though | 09:39 |
| rogpeppe | fwereade: you should be interested in this too BTW: https://docs.google.com/a/canonical.com/spreadsheet/ccc?key=0AghcHASXWzXmdHdOX2htQVJITkdaYVh4VnZ2Z0RrQWc#gid=0 | 09:39 |
| rvba | rogpeppe: okay, ta. I'll put X? then. | 09:40 |
| rogpeppe | rvba: sgtm | 09:40 |
| fwereade | rogpeppe, rvba: really briefly, because I'm out today: an account is essentially <reference to cloud info> + <auth info> + <possible overrides to cloud info> | 09:40 |
| fwereade | rogpeppe, oh, nice spreadsheet | 09:41 |
| rogpeppe | fwereade: so would the ec2 region be part of an account? | 09:41 |
| fwereade | rogpeppe, might be, if someone almost always wants to use (say) eu-west-1 | 09:42 |
| fwereade | rogpeppe, that's one of those ugly things that really needs to be available for override at env and account level | 09:43 |
| rogpeppe | fwereade: is the idea to be able to specify *any* attribute at account level? | 09:43 |
| fwereade | rogpeppe, I don't think we can have a coherent model without allowing that, upsetting though it may be | 09:44 |
| fwereade | rogpeppe, but I don't think it's too painful to just pass unknown keys up levels until they're found to be comprehensible | 09:44 |
| rogpeppe | fwereade: hmm, so it's not really "account" - it's more like "environment superclass" | 09:44 |
| fwereade | rogpeppe, I don't personally see that as a particularly useful conception of it, but I expect mileage legitimately varies | 09:45 |
| rogpeppe | fwereade: or, i guess "environment template" | 09:45 |
| fwereade | rogpeppe, no, I don't think so... | 09:46 |
| rogpeppe | fwereade: i don't really see anything particularly account-like about it, that's all | 09:46 |
| fwereade | rogpeppe, the env may contribute attrs to the account, not vice versa | 09:46 |
| fwereade | rogpeppe, the auth info is the critical bit | 09:46 |
| fwereade | rogpeppe, that's the stuff that has to be included at that level | 09:46 |
| rogpeppe | fwereade: erm, surely we're saying that the account can contribute attrs to the env too, no? | 09:46 |
| fwereade | rogpeppe, how is region anything to do with the environment? | 09:47 |
| fwereade | rogpeppe, it's all about the cloud | 09:47 |
| fwereade | rogpeppe, but it can be specified differently at more specific levels | 09:47 |
| rogpeppe | fwereade: ah | 09:47 |
| rogpeppe | fwereade: so we've really got three levels, "account", "cloud" and "environment" | 09:47 |
| fwereade | rogpeppe, it's definitely a surprising model from our existing perspective but it really holds together pretty well when you think about it enough, I think | 09:48 |
| rogpeppe | fwereade: but... are you conflating "account" and "cloud"? | 09:48 |
| rogpeppe | fwereade: i find it surprising that i can specify a region in something called an "account", which sounds like it's just got user credentials in it | 09:49 |
| rogpeppe | fwereade: it feels a bit like the responsibilities are mixed up there | 09:50 |
| fwereade | rogpeppe, the names indicate what must be specified, the other bits that *can* be specified are about convenience for users (and, yeah, some degree of irritating conceptual bleed for us developers) | 09:51 |
| rogpeppe | fwereade: i'm no longer sure how many levels of inheritance we're aiming for here | 09:52 |
| rogpeppe | fwereade: i'm not sure it's just a conceptual difficulty for developers - i'd find it awkward to explain to a user too. | 09:54 |
| fwereade | rogpeppe, it's not the environment inheriting from the account inheriting from the cloud -- it's the environment overwriting values on the account overwriting values on the cloud | 09:54 |
| rogpeppe | fwereade: i'm not sure i see the difference there | 09:54 |
| fwereade | rogpeppe, inheritance implies an env is an account is a cloud | 09:56 |
| fwereade | rogpeppe, it's rather more like embedding, I think | 09:56 |
| rogpeppe | fwereade: if i can specify all the env attrs on an account or a cloud, i'm not sure i see there is a difference | 09:56 |
| * fwereade is neglecting his family | 09:56 | |
| rogpeppe | fwereade: right, go away! :-) | 09:56 |
| rogpeppe | fwereade: sorry for distracting you | 09:57 |
| fwereade | rogpeppe, (when did I say you could specify an env attr on an account? I don't *think* it needs to flow that way too) | 09:57 |
| rogpeppe | fwereade: hmm, i'd thought the env ended up with all the attributes from the account | 09:58 |
| fwereade | rogpeppe, I think that the env ends up with an account (which may have had overrides applied to it), and the *env* only has a few juju-level things set -- one of which is the account | 09:59 |
| rogpeppe | fwereade: but are we allowing users to specify a "cloud" thing as well as an environment and an account? | 09:59 |
| fwereade | rogpeppe, because of private clouds essentially | 09:59 |
| fwereade | rogpeppe, most of the time all that tedious cloud stuff comes from simplestreams, so account specification is trivial | 09:59 |
| rogpeppe | fwereade: an account specifies a cloud too? | 10:00 |
| fwereade | rogpeppe, sometimes it needs to be hand-specified, and we (1) don't want to dirty up the account specification and (2) may well have a couple of accounts on the same cloud for say testing/staging/production | 10:00 |
| fwereade | rogpeppe, yes, account = <cloud reference> + <auth info> + <possible cloud overrides for convenience> | 10:01 |
| fwereade | rogpeppe, env = <account reference> + <juju info> + <possible account overrides for convenience> | 10:01 |
| * fwereade is really off now, and hopes the model makes some sort of sense | 10:02 | |
| * rogpeppe thinks about it | 10:02 | |
| rogpeppe | fwereade: thanks | 10:02 |
| rogpeppe | hmm, that's a first for me - laptop just spontaneously rebooted. i wasn't doing anything significant at the time | 10:33 |
| * rogpeppe tries to recollect all the threads | 10:33 | |
| dimitern | rogpeppe: it happens to me sometimes since I upgraded to raring - all due to video drivers | 10:34 |
| rogpeppe | dimitern: yeah, i reckon that's likely to be the problem | 10:35 |
| dimitern | rogpeppe: btw, should we have an EnvironConfig() call in the uniter API? the uniter uses it, but I remember there were some concerns about it | 10:35 |
| rogpeppe | dimitern: i also get a weird thing occasionally where everything goes ultra slowly | 10:35 |
| rogpeppe | dimitern: what is the uniter using it for? | 10:35 |
| * dimitern checks | 10:35 | |
| dimitern | rogpeppe: to get the provider type and from then the private address | 10:36 |
| dimitern | rogpeppe: and public | 10:36 |
| rogpeppe | dimitern: i think you should just have a ProviderType API call | 10:37 |
| dimitern | rogpeppe: ok | 10:37 |
| natefinch | morning all | 11:13 |
| rogpeppe | dimitern: here's the list of all the relation names used in the 193 charms i could find in the charm store, most frequent first: http://paste.ubuntu.com/5992327/ | 11:13 |
| rogpeppe | natefinch: hiya | 11:13 |
| rogpeppe | dimitern: looks like we can be quite restrictive without it being a problem | 11:14 |
| dimitern | rogpeppe: so it looks like ([a-z-]+)+ should be enough? | 11:15 |
| dimitern | rogpeppe: or more like ([a-z]+-?)+, except it shouldn't end with - | 11:16 |
| natefinch | mgz: I was looking at maas and azure for addressability. I have some code for azure, but it's pretty ugly... I couldn't find where maas was exposing IPs, though | 11:16 |
| rogpeppe | dimitern: i think i'd go for [a-z][a-z0-9]*(-[a-z0-9]+)* | 11:16 |
| dimitern | rogpeppe: sgtm | 11:17 |
| rogpeppe | dimitern: in case you're interested, i downloaded all the charms with this: http://paste.ubuntu.com/5992345/ (unfortunately obtaining the list of charms was a bit more hassle) | 11:18 |
| rogpeppe | dimitern: and i got it to spew out the relation names with this: http://paste.ubuntu.com/5992349/ | 11:18 |
| rogpeppe | dimitern: (i ran it with this command line: http://paste.ubuntu.com/5992351/) | 11:20 |
| rogpeppe | dimitern: so that command is also potentially useful for other charm metadata analysis we might want to do | 11:20 |
| dimitern | rogpeppe: cool! so you didn't miss anything | 11:20 |
| mgz | natefinch: for maas, you might want to ask one of the red squad guys | 11:20 |
| rogpeppe | dimitern: well, i might well have missed some charms | 11:20 |
| mgz | we need a different mechanism for the local provider currently, we might need one for maas too, but I think when I looked at it there seemed to be a reasonable way | 11:21 |
| mgz | standup guys? | 11:33 |
| geme_ | juju-core 1.13.1 won't bootstrap on private openstack. /var/log/cloud-init-output.log reports : ERROR juju open.go:89 state: connection failed, will retry: dial tcp 127.0.0.1:37017: connection refused. There's nothing running on this port.mongod is running on 27017 and 28017 though | 11:59 |
| mgz | geme_: you need to ssh into the state server machine and look at the juju logs | 12:07 |
| mgz | I had a similar issue a while back, but I'm failing to find or recall what the root cause was | 12:08 |
| geme_ | which log ? | 12:08 |
| mgz | look at things under /var/log/juju | 12:08 |
| mgz | there's also a mongo log that I remember looking at... | 12:10 |
| mgz | gah, I remember talking to jam about it, but can't find it in the chat logs anywhere | 12:13 |
| mgz | geme_: any luck? the first few lines of /var/log/juju/machine-0.log should be about mongo state connection | 12:18 |
| geme_ | mgz, there is no machine-0.log - just an empty all-machines.log | 12:20 |
| mgz | okay, so can you pastebin your whole cloud-init please? seems the agent didn't get started at all | 12:20 |
| geme_ | cloud-init.log and cloud-init-output.log ? | 12:21 |
| mgz | -output please | 12:22 |
| geme_ | mgz, jusr seen : 2013-08-16 10:36:05,970 - cc_apt_update_upgrade.py[WARNING]: Source Error: ppa:juju/experimental:add-apt-repository failed | 12:26 |
| mgz | what do you get if you sudo run thant command from the box now? | 12:27 |
| geme_ | mgz, http://paste.ubuntu.com/5992536/ | 12:33 |
| mgz | oh god, that statecert logging | 12:34 |
| mgz | geme_: okay, so it installs the wrong version of mongodb (2.0.4-1ubuntu2.1) because the repo add failed | 12:35 |
| geme_ | adding the repo works from the cmdline | 12:36 |
| geme_ | though it needs a return response | 12:37 |
| mgz | meh, and it's a bare except clause in cloudinit/config/cc_apt_update_upgrade.py that throws away the actual error when logging | 12:42 |
| geme_ | mgz, any idea why adding the repo failed ? | 12:42 |
| mgz | my best guess is somehow the networking isn't up yet so it fails trying to talk to launchpad | 12:43 |
| mgz | but could be anything, and after all the install works fine after that | 12:43 |
| mgz | one thing you could try is reproducing the error with `nova boot --user-data CLOUDCONFIGFILE` | 12:45 |
| geme_ | mgz, when cloud-init runs can it access envars like HTTP_PROXY ? | 12:46 |
| mgz | where cloud config is just something like a file containing "#cloud-config\napt-sources:\n - source: ppa:juju/experimental" (with real newlines etc) | 12:47 |
| mgz | geme_: set where? if you have some funky proxy setup for external net access, that is likely the issue | 12:47 |
| geme_ | mgz, /etc/bash.bashrc | 12:48 |
| mgz | doubt very much cloud-init will pick up that, except perhaps as a side-effect to some subcommands | 12:49 |
| geme_ | mgz, Is there another way to make these proxy setting available to cloud-init ? | 12:51 |
| mgz | I'm not even sure that's sufficient, not everything respects HTTP_PROXY | 12:51 |
| geme_ | mgz, so firewalls are a blocker then - sorry for the pun :) | 12:54 |
| mgz | not really, we've got a bunch of firewalled clouds that work fine | 12:54 |
| mgz | I don't think any try to do it using HTTP_PROXY though | 12:54 |
| mgz | which is an application level setting, if you're trying to inject that into your vms it doesn't seem a reliable way of doing things | 12:55 |
| mgz | you need to configure openstack to understand your network setup | 12:55 |
| geme_ | mgr, the proxy settings work in the shell | 12:57 |
| mgz | sure, because it's a shell | 12:57 |
| mgz | lots of things in a vm will not be spun up from bash | 12:57 |
| mgz | I think what you probably want is just a local apt proxy, but I'm still not sure how apt-add-reposistory interacts with that | 12:59 |
| mgz | you might want to try asking in #juju how people have set up juju on openstack clouds without outgoing access to the wider internet | 13:00 |
| mgz | because a bunch of guys in there have done that | 13:00 |
| geme_ | mgr, ok I'll ask. Thanks | 13:01 |
| rogpeppe | dimitern: i realised that i'd missed most charms in my download, but rick_h in juju-gui pointed me to the charmworld api, so here's a program that downloads *all* the charms: http://paste.ubuntu.com/5992628/ | 13:02 |
| rogpeppe | dimitern: 663 successfully acquired at last count | 13:02 |
| dimitern | rogpeppe: nice | 13:02 |
| rogpeppe | dimitern: and our relation regexp is still fine | 13:02 |
| dimitern | rogpeppe: great! | 13:02 |
| dimitern | rogpeppe: we should have some repo for such useful tools | 13:03 |
| rogpeppe | dimitern: yeah, they might well be useful to others | 13:03 |
| dimitern | rogpeppe: so if you have a peer relation, are the remote and local units of it the same? | 13:05 |
| rogpeppe | dimitern: no | 13:05 |
| dimitern | rogpeppe: why did you resolve all my comments on that document? | 13:06 |
| rogpeppe | dimitern: i turned then all into notes | 13:07 |
| rogpeppe | dimitern: which seemed a much more appropriate way of doing things | 13:07 |
| rogpeppe | dimitern: (i didn't know about notes when i started adding comments) | 13:07 |
| dimitern | rogpeppe: ha! I didn't know there is such a feature | 13:07 |
| rogpeppe | dimitern: thank rvba | 13:07 |
| dimitern | rvba: thanks :) | 13:07 |
| dimitern | rogpeppe: so I'm pondering how to get the remote unit from a relationunit | 13:08 |
| dimitern | rogpeppe: I can get the relation of the RU, call RelatedEndpoints on it, passing the local unit's service name | 13:08 |
| dimitern | rogpeppe: I'll a slice with length 1 (peer) or 2 (provider-requirer) with the endpoints | 13:09 |
| dimitern | rogpeppe: and I can get the remote service name from eps[0].ServiceName or eps[1].ServiceName respectively | 13:09 |
| dimitern | rogpeppe: but how to get which unit is on the other end? | 13:10 |
| dimitern | rogpeppe: /I'll a/I'll get a/ | 13:10 |
| rogpeppe | dimitern: an endpoint doesn't imply a unit | 13:11 |
| dimitern | rogpeppe: so there's no way to know which is the remote unit | 13:11 |
| rogpeppe | dimitern: the client will need to name the unit, i think | 13:11 |
| dimitern | rogpeppe: ok, so ReadSettings cannot be made to enforce "use remote unit only" | 13:12 |
| dimitern | rogpeppe: in fact I can see in test that it gets called for the local unit as well | 13:12 |
| rogpeppe | dimitern: well, there are many possible remote units | 13:12 |
| dimitern | rogpeppe: right.. | 13:12 |
| rogpeppe | dimitern: and we need to be able to read our own settings too | 13:12 |
| dimitern | rogpeppe: for that, you have another call | 13:13 |
| dimitern | rogpeppe: Settings() | 13:13 |
| rogpeppe | dimitern: what's the difference between the two? | 13:13 |
| dimitern | rogpeppe: Settings() returns a mutable *Settings, whereas ReadSettings() returns a plain map[string]interface{} | 13:14 |
| rogpeppe | dimitern: but they'll use the same underlying API call, right? | 13:14 |
| dimitern | rogpeppe: no | 13:18 |
| dimitern | rogpeppe: but perhaps they should actually | 13:18 |
| rogpeppe | dimitern: ah, what's the difference? | 13:18 |
| dimitern | rogpeppe: ReadSettings only | 13:18 |
| rogpeppe | dimitern: i think they're both doing the same thing | 13:18 |
| rogpeppe | dimitern: the difference is only at the client side | 13:18 |
| dimitern | rogpeppe: and it to simulate Settings() you need to call it with "relation-x", "unit-x", "unit-x" | 13:19 |
| dimitern | rogpeppe: I have defined a RelationUnitPair with Relation, LocalUnit and RemoteUnit string | 13:19 |
| dimitern | rogpeppe: I can make RemoteUnit to be optional | 13:20 |
| rogpeppe | dimitern: hmm, can't you just omit LocalUnit? | 13:20 |
| rogpeppe | dimitern: because it's implied by the client | 13:21 |
| dimitern | rogpeppe: no, let's not do that | 13:21 |
| dimitern | rogpeppe: if we're going to start omitting stuff because it's implied by the authenticated entity, that's a whole new can of worms | 13:21 |
| rogpeppe | dimitern: ok, fair enough | 13:22 |
| dimitern | rogpeppe: possibly making bulk calls a moot in general | 13:22 |
| rogpeppe | dimitern: ahem, yes | 13:22 |
| rogpeppe | dimitern: [don't get me started :-)] | 13:22 |
| dimitern | rogpeppe: so, until and if we decide to do that, I'll prefer to be explicit | 13:22 |
| rogpeppe | dimitern: so LocalUnit must always be the authenticated unit, right? | 13:22 |
| dimitern | rogpeppe: yes | 13:22 |
| rogpeppe | dimitern: so i think there's no need to make RemoteUnit optional | 13:23 |
| rogpeppe | dimitern: if you want to get settings of the remote unit, just have RemoteUnit==LocalUnit | 13:23 |
| dimitern | rogpeppe: so pass both and make them the same to request local settings | 13:23 |
| dimitern | rogpeppe: ok | 13:23 |
| dimitern | rogpeppe: no, it's exactly the opposite :) | 13:23 |
| rogpeppe | dimitern: then RemoteUnit is always the tag of the unit you're asking for settings of | 13:23 |
| dimitern | rogpeppe: Local!=Remote => remote settings | 13:23 |
| rogpeppe | dimitern: ha, yes, typo | 13:24 |
| rogpeppe | dimitern: if you want to get settings of the *local* unit... ! | 13:24 |
| dimitern | rogpeppe: ok then, almost done - will propose soon | 13:24 |
| rogpeppe | dimitern: cool | 13:24 |
| dimitern | rogpeppe: remind me again why you removed the unitTagToName conversion from names? | 13:28 |
| rogpeppe | dimitern: you've got ParseTag now | 13:28 |
| dimitern | rogpeppe: ah, right | 13:28 |
| rogpeppe | dimitern: when you've a moment, try running "go get launchpad.net/juju-utils/cmd/getallcharms" | 13:38 |
| rogpeppe | dimitern: there's also launchpad.net/juju-utils/cmd/charmmeta | 13:39 |
| geme_ | mgz, added repo to the image. Now bootstraps successfully and juju status respond. Likely the problem will come back and bite me. | 13:39 |
| dimitern | rogpeppe: done both | 13:39 |
| rogpeppe | dimitern: you could try and see if it works | 13:40 |
| rogpeppe | dimitern: e.g. getallcharms ~/allcharms | 13:40 |
| dimitern | rogpeppe: trying now | 13:40 |
| rogpeppe | dimitern: it should fetch all the charms in the charm store | 13:40 |
| dimitern | rogpeppe: is there a -v option? | 13:40 |
| dimitern | rogpeppe: ah, it dumps them | 13:40 |
| rogpeppe | dimitern: yeah. it's actually verbose by default | 13:41 |
| dimitern | rogpeppe: for some it reports 2013/08/16 15:40:58 get "cs:~philipballew/precise/dyndns-0" failed: charm "noip2" relation "dynamic-ip-reporting" using a reserved interface: "juju-info" | 13:41 |
| rogpeppe | dimitern: yup, that's the most common issue | 13:41 |
| dimitern | rogpeppe: done, 361M fetched | 13:42 |
| rogpeppe | dimitern: cool | 13:42 |
| dimitern | rogpeppe: and charmmeta ? | 13:42 |
| dimitern | rogpeppe: it's not recursive | 13:43 |
| rogpeppe | dimitern: now you can run charmmeta over all of them by doing something like: charmmeta sometemplatetext $(cat $HOME/allcharms/urls.txt) | 13:43 |
| rogpeppe | dimitern: no - it probably should recurse looking for metadata.yaml files | 13:44 |
| rogpeppe | dimitern: but i've spent too much time on it already :-) | 13:45 |
| dimitern | rogpeppe: np, with some tweaking it works | 13:45 |
| rogpeppe | dimitern: what tweaking? | 13:45 |
| dimitern | rogpeppe: charmmeta '..template..' `cat ~/allcharms/urls.txt` fails | 13:46 |
| dimitern | rogpeppe: it reports Dir==nil on each line | 13:46 |
| rogpeppe | dimitern: ah! | 13:47 |
| rogpeppe | dimitern: that's because you need to be inside the allcharms directory | 13:47 |
| dimitern | rogpeppe: yeah, I figured :) | 13:47 |
| dimitern | man.. testing Enter and LeaveScope is a bitch | 13:49 |
| dimitern | I have to use a scope watcher.. | 13:49 |
| rogpeppe | dimitern: charmmeta is now recursive. couldn't quite resist doing it now :-) | 14:04 |
| dimitern | rogpeppe: cool, will test it | 14:07 |
| * rogpeppe must get some lunch now | 14:11 | |
| === tasdomas is now known as tasdomas_afk | ||
| rogpeppe | trivial review, anyone? https://codereview.appspot.com/13053043 | 15:07 |
| rogpeppe | just some package renaming, mechanical changes | 15:08 |
| natefinch | rogpeppe: I can review it | 15:15 |
| rogpeppe | natefinch: thanks | 15:15 |
| natefinch | rogpeppe: reviewed. LGTM, bonus points for fixing a few of the import sorts | 15:23 |
| rogpeppe | natefinch: good catch | 15:23 |
| rogpeppe | dimitern, mgz, rvba, jtv, fwereade: another review appreciated please. trying to avoid a prereq. https://codereview.appspot.com/13053043 | 15:30 |
| dimitern | rogpeppe: looking | 15:33 |
| rogpeppe | dimitern: ta | 15:33 |
| rogpeppe | dimitern: i okayed this with fwereade previously, BTW | 15:33 |
| dimitern | rogpeppe: while I'm at it, would you look at https://codereview.appspot.com/13055043 | 15:34 |
| rogpeppe | dimitern: looking | 15:34 |
| dimitern | rogpeppe: thanks | 15:36 |
| dimitern | rogpeppe: didn't we discuss having the providers at top-level? | 15:36 |
| rogpeppe | dimitern: they're very much to do with environs | 15:36 |
| rogpeppe | dimitern: (they implement an interface defined in environs) | 15:36 |
| rogpeppe | dimitern: so i strongly feel they below below environs | 15:37 |
| rogpeppe | dimitern: (as does instance, but that's another story) | 15:37 |
| dimitern | rogpeppe: that shouldn't stops us - there are cases where other packages implement interfaces defined outside their parent package | 15:37 |
| rogpeppe | dimitern: yes, but really environs is all about providers | 15:37 |
| rogpeppe | dimitern: hmm, well, i can see your p.o.v. too | 15:38 |
| dimitern | rogpeppe: providers, like charms are things that need an environment | 15:38 |
| rogpeppe | dimitern: not really: providers *provide* an environment | 15:39 |
| dimitern | rogpeppe: but really the environment needs both of them | 15:39 |
| rogpeppe | dimitern: well, they provide an Environ anyway | 15:39 |
| dimitern | rogpeppe: the environment is much more that what a provider provides | 15:39 |
| dimitern | rogpeppe: it provides means to access the environment, but not the only means | 15:39 |
| rogpeppe | dimitern: a provider provides an Environ | 15:40 |
| dimitern | rogpeppe: state and API are other means to access it | 15:40 |
| rogpeppe | dimitern: and that's what environs is all about | 15:40 |
| dimitern | rogpeppe: well, Environ is hardly the best name for it | 15:40 |
| rogpeppe | dimitern: state and API don't really have anything to do with Environ | 15:40 |
| rogpeppe | dimitern: that may be true | 15:40 |
| dimitern | rogpeppe: i think we're conflating the Environ type and what a juju environment is all about | 15:41 |
| dimitern | rogpeppe: the Environ type is *not* a juju environment | 15:41 |
| rogpeppe | dimitern: i didn't say it was | 15:41 |
| rogpeppe | dimitern: well, | 15:41 |
| rogpeppe | dimitern: it depends what you mean by a juju environment | 15:41 |
| dimitern | rogpeppe: I mean both local and remote stuff | 15:42 |
| rogpeppe | dimitern: but regardless, the provider types are there entirely to implement the interface defined in environs/interface.go | 15:42 |
| dimitern | rogpeppe: settings, instances, cloud account info | 15:42 |
| rogpeppe | dimitern: they aren't accessed in any way accept through environs | 15:42 |
| rogpeppe | dimitern: so i think it's reasonable to put them under that directory | 15:43 |
| dimitern | rogpeppe: they are - through the CLI and the API | 15:43 |
| rogpeppe | dimitern: not really. whenever the CLI gets an Environ, it gets it via the environs package | 15:43 |
| dimitern | rogpeppe: should we put the API in environs as well, because all it does is to provide access to a running environment's parts? | 15:43 |
| rogpeppe | dimitern: no, an Environ (whatever it should really be called) is all about encapsulating provider-specific functionality | 15:44 |
| dimitern | rogpeppe: I really liked the way pyjuju has providers at top level | 15:44 |
| rogpeppe | s/whatever/or whatever/ | 15:44 |
| dimitern | rogpeppe: and the argument that providers need to implement an interface in environs/interface.go is not a strong point to keep them together IMO | 15:45 |
| dimitern | rogpeppe: we have plenty of cases where that's not true | 15:46 |
| rogpeppe | dimitern: if they provided independent functionality too, i might agree | 15:46 |
| rogpeppe | dimitern: but given that nothing else in the system imports them directly (except for the side effect of having them there, and excepting dummy which is special), i think it's reasonable to hide them under environs | 15:47 |
| dimitern | rogpeppe: how about the null provider? | 15:47 |
| dimitern | rogpeppe: it's not a complete provider - also possibly the ssh one | 15:48 |
| rogpeppe | dimitern: i don't know what the null provider is. have we got one of those? | 15:48 |
| dimitern | rogpeppe: we will soon | 15:48 |
| dimitern | rogpeppe: read the specs | 15:48 |
| dimitern | rogpeppe: it's an example of a "partial" provider | 15:49 |
| dimitern | rogpeppe: you cannot start or stop instances for example | 15:49 |
| dimitern | rogpeppe: or xenv rels? | 15:49 |
| rogpeppe | dimitern: it will still need to implement all the methods defined in environs.EnvironProvider | 15:49 |
| rogpeppe | dimitern: even if it returns errors for some of them | 15:49 |
| dimitern | rogpeppe: I'm saying this might change | 15:50 |
| rogpeppe | dimitern: i'm not sure it needs to | 15:50 |
| dimitern | rogpeppe: we might need to separate that huge interface into smaller ones | 15:50 |
| rogpeppe | dimitern: and we can change things again when that happens | 15:50 |
| rogpeppe | dimitern: i plan to do just that | 15:50 |
| dimitern | rogpeppe: well.. | 15:50 |
| rogpeppe | dimitern: but i'm still happy to have them all under environs | 15:51 |
| dimitern | rogpeppe: ok, at least havingthem in environs/provider/ will clean up some of the mess already in there | 15:51 |
| dimitern | rogpeppe: the same should apply to cloudinit imo | 15:51 |
| rogpeppe | dimitern: where should environs/cloudinit go? | 15:51 |
| dimitern | rogpeppe: in cloudinit/ | 15:51 |
| rogpeppe | dimitern: cloudinit/cloudinit ? | 15:52 |
| rogpeppe | dimitern: (there's already a juju-core/cloudinit package) | 15:52 |
| dimitern | rogpeppe: I know | 15:52 |
| dimitern | rogpeppe: why shouldn't it be in the same top-level package? | 15:52 |
| dimitern | rogpeppe: why the separation? | 15:52 |
| rogpeppe | dimitern: because the top level package is entirely juju-agnostic | 15:53 |
| rogpeppe | dimitern: whereas environs/cloudinit is intimately bound up with juju environment-related stuff | 15:53 |
| rogpeppe | dimitern: and that's kinda where my reasoning for when we should have a top level package or namespace comes from - it should be largely independent functionality. | 15:54 |
| dimitern | rogpeppe: anyway, that's entirely another topic in its own right | 15:54 |
| rogpeppe | dimitern: the cloudinit package works well as something standalone. environs/cloudinit builds neatly on top of it. | 15:55 |
| dimitern | rogpeppe: if it's independent it shouldn't be in juju-core - it should be an outside package | 15:55 |
| rogpeppe | dimitern: perhaps. but if we don't care about other people using them, it's more convenient to keep it in juju-core | 15:55 |
| rogpeppe | dimitern: we can still have decent modularity, even inside juju-core | 15:56 |
| dimitern | rogpeppe: if it's useful independently, it shouldn't be inside | 15:56 |
| rogpeppe | dimitern: there are lots of pieces that are useful independently. having them outside is a burden we don't want to have to bear for all of them | 15:56 |
| rogpeppe | dimitern: schema and rpc are two examples | 15:57 |
| rogpeppe | dimitern: anyway, anyone outside can still use these things | 15:57 |
| rogpeppe | dimitern: they don't need to be in an independent package to enable external reuse | 15:57 |
| dimitern | rogpeppe: I don't quite agree the burden is that big | 15:58 |
| dimitern | rogpeppe: most of this things are stable and change rarely | 15:59 |
| rogpeppe | dimitern: i don't think there's that much in the way of advantage from having them in separate projects | 15:59 |
| dimitern | rogpeppe: actually that could be a reason to move it out | 15:59 |
| dimitern | rogpeppe: of course there is - smaller code base, easier to maintain | 15:59 |
| rogpeppe | dimitern: i don't think the maintenance burden is any smaller if the code is separated across several projects | 16:00 |
| rogpeppe | dimitern: the opposite if anything | 16:00 |
| dimitern | rogpeppe: there's only a burden if we have to update them often - the same thing as with gwacl, goose, etc. | 16:00 |
| rogpeppe | dimitern: if we don't need to update them, where's the burden from having them in the same code base? | 16:01 |
| rogpeppe | dimitern: i think it's great to build a project from many small indpendent parts | 16:01 |
| rogpeppe | dimitern: and if those parts were principally built for the main project, i don't really see there's an advantage to moving them out | 16:02 |
| rogpeppe | dimitern: the main advantage of doing that is if we want to promote external users of those packages | 16:02 |
| dimitern | rogpeppe: it's not a burden as such to have them in the same codebase, it just inflates it, and if they rarely change, it's actually better and cleaner to have them outside | 16:03 |
| rogpeppe | dimitern: i guess i don't see that. the more external dependencies, the more hassle. | 16:03 |
| rogpeppe | dimitern: otherwise we'd have one project for each package | 16:04 |
| dimitern | rogpeppe: what's the hassle exactly? | 16:05 |
| rogpeppe | dimitern: moving the code base to different versions doesn't automatically update the dependencies | 16:05 |
| rogpeppe | dimitern: i can see the history of those dependencies in with the main log | 16:06 |
| rogpeppe | dimitern: if they're inside the code base | 16:06 |
| dimitern | rogpeppe: aren't we moving towards handling dependencies better in general? | 16:06 |
| rogpeppe | dimitern: yeah, but it's still not cost-free | 16:06 |
| dimitern | rogpeppe: it depends on how easy we make it | 16:07 |
| dimitern | rogpeppe: nothing is cost free, there are hidden costs you have to consider | 16:08 |
| rogpeppe | dimitern: back to the environs/provider thing: for me, it's about strict encapsulation boundaries. the environs package should entirely encapsulate the different providers. | 16:08 |
| rogpeppe | dimitern: hidden costs like? | 16:08 |
| dimitern | rogpeppe: reviewed | 16:09 |
| rogpeppe | dimitern: i don't want to do any import renaming in that branch if that's ok. i can go through in another branch doing all the gocheck renames if you'd like | 16:10 |
| dimitern | rogpeppe: like having a bunch of code which is not directly juju-related, only slightly and could be reused by external projects | 16:11 |
| rogpeppe | dimitern: external projects can still reuse that code | 16:11 |
| dimitern | rogpeppe: Ok, at least fix the imports separation then - they were just a few | 16:12 |
| rogpeppe | dimitern: (and does - for instance that charmmeta command) | 16:12 |
| rogpeppe | dimitern: will do, thanks | 16:12 |
| dimitern | rogpeppe: i don't want to argue anymore about this - it deserves a separate discussion in good time :) | 16:13 |
| rogpeppe | dimitern: indeed :-) | 16:13 |
| natefinch | dimitern, rogpeppe: FWIW I agree with both of you... both ways have advantages and disadvantages. It's a tale as old as time (or at least as old as modern programming) | 16:13 |
| dimitern | natefinch: nah.. c'mon you can't agree like that :) | 16:15 |
| natefinch | dimitern: what, I have to take a side? :) | 16:16 |
| dimitern | natefinch: hehe | 16:16 |
| dimitern | natefinch: you can have a look at this perhaps? https://codereview.appspot.com/13055043/ | 16:16 |
| natefinch | dimitern: sure | 16:18 |
| dimitern | natefinch: ta | 16:19 |
| natefinch | dimitern: just curious, why do we do "type RelationUnits struct { RelationUnits []RelationUnit}" rather than "type RelationUnits []RelationUnit" ? | 16:23 |
| dimitern | natefinch: because the rpc layer of the API needs a struct as args and results | 16:23 |
| natefinch | dimitern: ahh, ok | 16:24 |
| dimitern | natefinch: take a look at doc/api.txt and rpc/server.go -> Serve()'s doc comment you're interested in the internals | 16:25 |
| dimitern | fwereade_: hey | 16:25 |
| dimitern | fwereade_: just in time for a review? :) | 16:25 |
| dimitern | natefinch: thanks | 16:51 |
| dimitern | natefinch: btw - I'd like to leave the checkers.IsTrue etc. for a follow-up, if you don't mind. The diff is already big enough | 16:52 |
| dimitern | rogpeppe: does it look roughly ok? | 16:56 |
| rogpeppe | dimitern: i'm working on doing all the gc.etc stuff in about 4 giant branches | 16:56 |
| rogpeppe | dimitern: i think i can do most of the changes automatically | 16:57 |
| dimitern | rogpeppe: good to hear | 16:57 |
| dimitern | rogpeppe: but please don't forget my branch as well ;) | 16:57 |
| rogpeppe | dimitern: i'm looking now | 16:58 |
| dimitern | rogpeppe: cheers | 16:59 |
| natefinch | rogpeppe: I was looking at how to do the . to gc change automatically... did you find a good way to do it? | 17:03 |
| rogpeppe | natefinch: i've got a command that may help - we'll see if it's up to the task | 17:03 |
| rogpeppe | natefinch: it's pretty much a failed experiment, but still sometimes useful | 17:04 |
| natefinch | rogpeppe: yeah, I looked around for something that might already do it, gofix or something, but nothing seemed to be able to do it, so I was looking into writting a command to do it by parsing the AST... hopefully yours will work so I don't have to do that :) | 17:04 |
| rogpeppe | natefinch:code.google.com/p/rog-go/exp/cmd/gosym | 17:04 |
| rogpeppe | natefinch: gofmt isn't far off being able to do it | 17:05 |
| natefinch | rogpeppe: yeah, I looked at gofmt too... wasn't sure how to get it to figure out what is getting imported by . and fix it automatically.... at least in the general case | 17:06 |
| rogpeppe | natefinch: it can't do that | 17:06 |
| rogpeppe | natefinch: it needs to resolve type symbols | 17:06 |
| rogpeppe | natefinch: gosym resolves symbols but it doesn't cope with import to . well | 17:07 |
| natefinch | rogpeppe: I'm honestly surprised import . even exists in Go. Seems like the kind of "almost always a bad idea" thing that it would not support | 17:09 |
| rogpeppe | natefinch: it's only there for a particular edge case | 17:09 |
| rogpeppe | natefinch: but it's been abused | 17:09 |
| rogpeppe | dimitern: can settings attributes be deleted? | 17:14 |
| dimitern | rogpeppe: they are replaced | 17:15 |
| dimitern | rogpeppe: so yeah | 17:15 |
| rogpeppe | dimitern: hmm, it looks like relation-set calls settings.Delete | 17:15 |
| dimitern | rogpeppe: perhaps an additional test is needed | 17:15 |
| rogpeppe | dimitern: but it looks as your API doesn't cater for deletions | 17:16 |
| dimitern | rogpeppe: settings.Delete is only local | 17:16 |
| rogpeppe | dimitern: it's possible that's not a problem though | 17:16 |
| dimitern | rogpeppe: any changes are always written with Settings.Write() | 17:16 |
| rogpeppe | dimitern: but if you've called Delete, Settings.Write will actually delete the attribute | 17:17 |
| rogpeppe | dimitern: it doesn't just overwrite it with nil | 17:17 |
| dimitern | rogpeppe: the settings are one blob, right? | 17:18 |
| rogpeppe | dimitern: no | 17:18 |
| rogpeppe | dimitern: the settings are held as mongo fields | 17:18 |
| rogpeppe | dimitern: each settings gets its own field | 17:18 |
| rogpeppe | s/settings/setting/ | 17:18 |
| rogpeppe | dimitern: look in state/settings.go | 17:18 |
| dimitern | rogpeppe: so what happens when you overwrite it with some different map? | 17:18 |
| rogpeppe | dimitern: it changes only the ones that have locally changed since you last read the settings | 17:19 |
| rogpeppe | dimitern: it unsets the ones that have been deleted and sets the others | 17:19 |
| rogpeppe | dimitern: it might be reasonable to say that setting a value to nil will be treated as a deletion | 17:20 |
| dimitern | rogpeppe: ok, will add more tests to make sure what I can read from the unit after calling WriteSettings is exactly as passed in the args | 17:21 |
| rogpeppe | dimitern: how will attributes get deleted? | 17:21 |
| dimitern | rogpeppe: by calling Update before Write? | 17:21 |
| rogpeppe | dimitern: Update never deletes an attribute | 17:21 |
| dimitern | rogpeppe: so you're saying the operation has to be more sophisticated | 17:22 |
| rogpeppe | dimitern: i think so... assuming we care about deleting attributes | 17:23 |
| dimitern | rogpeppe: reading old settings, finding what to delete, updating the rest, finally writing the result | 17:23 |
| rogpeppe | dimitern: no | 17:23 |
| dimitern | rogpeppe: no? | 17:23 |
| rogpeppe | dimitern: the API call has to be able to specify which settings are to be deleted | 17:23 |
| dimitern | rogpeppe: delete just does delete(c.core, key) | 17:24 |
| dimitern | rogpeppe: it doesn't seem it keeps the deleted ones separately | 17:24 |
| rogpeppe | dimitern: it infers which ones are deleted by looking at the difference between c.core and c.disk | 17:24 |
| dimitern | rogpeppe: assuming we start with an empty map, update with the new settings, then write should take care of deletions as well | 17:24 |
| rogpeppe | dimitern: what happens if we do: Update({"x": "y"}) then Update({}) ? | 17:25 |
| dimitern | rogpeppe: will delete everything | 17:25 |
| rogpeppe | dimitern: nope | 17:25 |
| rogpeppe | dimitern: "x" will still be around | 17:25 |
| dimitern | rogpeppe: no, I mean we can't do that really | 17:25 |
| dimitern | rogpeppe: Update is client-side only | 17:26 |
| dimitern | rogpeppe: on the API level we have read and write only | 17:26 |
| rogpeppe | dimitern: it would be possible to make Update overwrite all attributes | 17:26 |
| rogpeppe | dimitern: but i don't think that's a great idea | 17:26 |
| dimitern | rogpeppe: how? | 17:27 |
| rogpeppe | dimitern: because it means potentially quite a bit more network traffic | 17:27 |
| dimitern | rogpeppe: sorry, I don't get you | 17:27 |
| dimitern | rogpeppe: we have old settings, we want to write new ones we got over the wire | 17:27 |
| rogpeppe | dimitern: if a hook just does a single "relation-set x=y", we don't really want to send all the other settings too | 17:27 |
| dimitern | rogpeppe: calling Read() will reset the disk cache, then we reset all core values, update the new ones, then write | 17:28 |
| rogpeppe | dimitern: are you talking client- or server-side here? | 17:28 |
| dimitern | rogpeppe: server-side | 17:28 |
| dimitern | rogpeppe: client-side we implement a Settings proxy object with a subset of the interface that state.Settings has, but only Write calls the API | 17:29 |
| dimitern | rogpeppe: the overhead is not significant | 17:29 |
| rogpeppe | dimitern: if we replace all core values, the client must send all those values up with the Write call | 17:29 |
| dimitern | rogpeppe: yes it will | 17:29 |
| dimitern | rogpeppe: the client calls Write eventually | 17:29 |
| dimitern | rogpeppe: in context, if I'm not mistaken, after the hook is comitted | 17:30 |
| dimitern | committed | 17:30 |
| dimitern | rogpeppe: yep, context.go:290 | 17:30 |
| rogpeppe | dimitern: are you happy that at the end of the hook, all settings will be sent across the network, even if they have not changed? | 17:30 |
| dimitern | rogpeppe: I see no problem with that | 17:30 |
| dimitern | rogpeppe: the settings are usually just a few | 17:31 |
| rogpeppe | dimitern: i don't know - have you looked at how charms use settings? | 17:31 |
| dimitern | rogpeppe: a few yes | 17:31 |
| rogpeppe | dimitern: i think it's quite possible that people have some large values in there sometimes | 17:31 |
| dimitern | rogpeppe: haven't look through all of them | 17:31 |
| rogpeppe | dimitern: we're not talking about config settings | 17:31 |
| dimitern | rogpeppe: can we check that reliably? | 17:32 |
| dimitern | rogpeppe: find all relation-set calls in all charms? | 17:32 |
| rogpeppe | dimitern: no - i just think it's not hard to be a little more efficient | 17:32 |
| dimitern | rogpeppe: like how? | 17:32 |
| dimitern | rogpeppe: have UpdateSettings and DeleteSettings calls? | 17:32 |
| dimitern | rogpeppe: pass every client-side call over the wire? | 17:33 |
| rogpeppe | dimitern: we could treat a nil value as a delete | 17:33 |
| rogpeppe | dimitern: so to delete an attribute you'd do: WriteSettings({"x": nil}) for example | 17:33 |
| dimitern | rogpeppe: deletes are not a problem | 17:33 |
| dimitern | rogpeppe: you're talking about something different | 17:33 |
| dimitern | rogpeppe: having incremental updates | 17:34 |
| rogpeppe | dimitern: afaics, deletes are the only problem with the current scheme | 17:34 |
| rogpeppe | dimitern: the current scheme will work fine to do incremental updates | 17:34 |
| rogpeppe | dimitern: the only exception being that you won't be able to delete an attribute | 17:34 |
| dimitern | rogpeppe: the current scheme will change as described and deletes won't be a problem anymore - it'll overwrite all stuff on write | 17:34 |
| dimitern | rogpeppe: not really | 17:35 |
| dimitern | rogpeppe: we're removed from the source when using the API | 17:35 |
| dimitern | rogpeppe: we can't rely on Update to do things smartly | 17:35 |
| dimitern | rogpeppe: we read the settings every time when we're about to write them | 17:35 |
| dimitern | rogpeppe: so there's no cache, etc. | 17:36 |
| rogpeppe | dimitern: i thought the idea was to replicate some similar Settings logic client side | 17:36 |
| dimitern | rogpeppe: yeah, except for Write | 17:36 |
| dimitern | rogpeppe: but Write will still overwrite what's currently in state | 17:36 |
| rogpeppe | dimitern: so that the client would read settings, make some modifications, then call Write, which could intelligently decide which have changed and issue an appropriate Update API call | 17:36 |
| dimitern | rogpeppe: I'm not keen on the idea of having an Update API call | 17:37 |
| rogpeppe | dimitern: why not? | 17:37 |
| dimitern | rogpeppe: it seems too much complexity for very marginal gain | 17:37 |
| rogpeppe | dimitern: in that case, there's no point in duplicating any of the Settings logic in the client side. | 17:38 |
| rogpeppe | dimitern: the uniter may as well just use map[string]interface{} directly | 17:38 |
| dimitern | rogpeppe: it being "in the odd case some charm writes huge settings keys once and changes a few smaller ones often" | 17:38 |
| dimitern | rogpeppe: well, it doesn't and changing the uniter is not necessary | 17:38 |
| dimitern | rogpeppe: the client-side will replicate the same logic with a proxy Settings | 17:39 |
| dimitern | rogpeppe: but on Write it'll overwrite current settings | 17:39 |
| dimitern | rogpeppe: and it will still work the same | 17:39 |
| dimitern | rogpeppe: update and delete will operate on the proxy object's internal map | 17:39 |
| dimitern | rogpeppe: write will send the final, merged map for saving | 17:39 |
| rogpeppe | dimitern: you'd only need one map, of course. | 17:40 |
| dimitern | rogpeppe: yeah | 17:40 |
| dimitern | rogpeppe: I can add a TODO in WriteSettings saying something like "if this proves to be inefficient we can change it to allow incremental updates" ? | 17:41 |
| rogpeppe | dimitern: how are you going to reset all attributes, BTW? | 17:42 |
| dimitern | rogpeppe: in fact, having huge values in settings keys is a possible DoS attack on mongo from a malicious charm | 17:42 |
| dimitern | rogpeppe: get the old ones, delete everything, write, update, write again is the easiest way that comes to mind | 17:43 |
| dimitern | rogpeppe: but perhaps there's a more efficient way, have to experiment a bit, to do it with a single write | 17:43 |
| rogpeppe | dimitern: i'm not sure there's a "delete everything" mongo operation | 17:43 |
| dimitern | rogpeppe: there is $unset deletions | 17:44 |
| dimitern | rogpeppe: that's what Write does | 17:44 |
| rogpeppe | dimitern: yes, but you have to know what attributes to unset | 17:44 |
| rogpeppe | dimitern: and the way you're using Settings, unset will never happen | 17:45 |
| dimitern | rogpeppe: all of them, which do not conflict with the new ones, which are simply updated | 17:45 |
| rogpeppe | dimitern: how do you know the "all"? | 17:45 |
| dimitern | rogpeppe: yes, we already discussed that, I change what's being done on WriteSettings | 17:45 |
| dimitern | rogpeppe: there's Keys() | 17:46 |
| dimitern | rogpeppe: or, perhaps simpler - get and delete each key not in the new map | 17:46 |
| dimitern | rogpeppe: just delete actually should do it | 17:46 |
| rogpeppe | dimitern: ISTM like the state.Settings object is actually getting in the way here. | 17:47 |
| dimitern | rogpeppe: after doing Read to populate the disk cache | 17:47 |
| dimitern | rogpeppe: how so? | 17:47 |
| rogpeppe | dimitern: because you'll be duplicating the same kind of logic that's already inside Settings | 17:47 |
| dimitern | rogpeppe: not much - just a for loop for deletions, a read before it, and update + write after it | 17:48 |
| rogpeppe | dimitern: part of the reason i'm slightly concerned here is that i suspect that these settings writes may be a good proportion of the ongoing mongo traffic in a large juju installation | 17:48 |
| dimitern | rogpeppe: yeah, and how is my approach different from what's already happening? | 17:49 |
| dimitern | rogpeppe: we always read them to create a settings object, do some ops on it, and call write finally - only the first and last operations are actually mongo ops | 17:50 |
| rogpeppe | dimitern: currently for each hook execution, we only have two mongo ops: read then write | 17:50 |
| rogpeppe | dimitern: we're going to change that so that it reads twice | 17:50 |
| rogpeppe | dimitern: and also currently, we read the settings, but we only write the data for the ones that have changed | 17:51 |
| dimitern | rogpeppe: not really, we can change the settings object to provide ClearCache method | 17:51 |
| rogpeppe | dimitern: how will that work? | 17:51 |
| dimitern | rogpeppe: that way we'll still have 1 read and 1 write | 17:51 |
| rogpeppe | dimitern: i don't know how if there's a mongo operation to delete all attributes on a doc without knowing what those attributes are. | 17:52 |
| dimitern | rogpeppe: it doesn't matter how many keys we set or unset - it's still a single update operation | 17:52 |
| dimitern | rogpeppe: I'm not talking about a mongo operation | 17:52 |
| dimitern | rogpeppe: I'm talking about ClearCache method which clears the settings object's internal disk cache | 17:52 |
| dimitern | rogpeppe: the same one write uses to determine which keys to unset | 17:53 |
| rogpeppe | dimitern: uniter.go:725 reads all the settings; uniter.go:728 writes them | 17:53 |
| rogpeppe | dimitern: every hook execution will call ReadSettings (which reads all the settings), then WriteSettings (which reads all the settings then writes them) | 17:53 |
| dimitern | rogpeppe: can we please stop discussing what's there now and focus on what we want to have there? | 17:54 |
| dimitern | rogpeppe: that's what I'm trying to explain | 17:54 |
| rogpeppe | dimitern: ok, so i think we still want to have ReadSettings followed by WriteSettings, right? | 17:54 |
| dimitern | rogpeppe: yes | 17:54 |
| dimitern | rogpeppe: it's the same as now | 17:54 |
| rogpeppe | dimitern: so the question is: how does WriteSettings delete any attributes which aren't in the map we're writing? | 17:54 |
| dimitern | rogpeppe: RU.Settings() does readSettings, Settings.Write() does the write | 17:55 |
| rogpeppe | dimitern: right - so we'll still have exactly the situation i described above, no? | 17:55 |
| dimitern | rogpeppe: by deleting them from the map, unless they are to be overwritten by keys in the new map | 17:55 |
| rogpeppe | dimitern: i.e. we'll read all the settings twice, then write them | 17:56 |
| dimitern | rogpeppe: ah, right, but in 2 separate API calls | 17:56 |
| rogpeppe | dimitern: yes | 17:56 |
| rogpeppe | dimitern: but the increase in mongo traffic is real | 17:57 |
| dimitern | rogpeppe: read does read, write does read+delete+update+write | 17:57 |
| dimitern | rogpeppe: how can you measure that? | 17:57 |
| rogpeppe | dimitern: write *should* just do a single mongo update transaction, i think | 17:57 |
| rogpeppe | dimitern: and i don't think it's at all hard to arrange that | 17:57 |
| dimitern | rogpeppe: we can't avoid the read in WriteSettings | 17:58 |
| rogpeppe | dimitern: yeah, i think we can | 17:58 |
| dimitern | rogpeppe: we don't have the orinal settings object around anymore | 17:58 |
| rogpeppe | dimitern: we'll just have to bypass the Settings object entirely. | 17:58 |
| rogpeppe | dimitern: it's not really appropriate for the server side | 17:59 |
| rogpeppe | dimitern: however.... | 17:59 |
| dimitern | rogpeppe: ah, so you're saying implment state.WriteSettings that does all that in one go? | 17:59 |
| rogpeppe | dimitern: yeah. or probably RU.WriteSettings | 17:59 |
| dimitern | rogpeppe: ok, that sounds reasonable | 17:59 |
| rogpeppe | dimitern: however... perhaps we could leave this as a TODO | 18:00 |
| dimitern | rogpeppe: and how will that work internally, without knowing what keys are already there? | 18:00 |
| rogpeppe | dimitern: we treat nil elements in the map passed to WriteSettings as deletions | 18:00 |
| rogpeppe | dimitern: the client side keeps track of deletions and passes them through as nils | 18:01 |
| dimitern | rogpeppe: that seems worse traffic-wise | 18:01 |
| rogpeppe | dimitern: only if deletions are common | 18:01 |
| dimitern | rogpeppe: now, instead of sending only stuff we actually want to store, we'll send also deleted things | 18:02 |
| rogpeppe | dimitern: we'll be sending deltas | 18:02 |
| rogpeppe | dimitern: if there are more than a few attributes that aren't changed, then it'll definitely be a win | 18:02 |
| dimitern | rogpeppe: yeah, that's one of those questions with no easy answers without having to go through every charm hook | 18:02 |
| rogpeppe | dimitern: yeah | 18:02 |
| rogpeppe | dimitern: it would be great to have some operational data to inspect | 18:03 |
| rogpeppe | dimitern: i really have to go, i'm afraid | 18:03 |
| dimitern | rogpeppe: ok, so: 1) treat key->nil as deletions, 2) update the rest | 18:03 |
| rogpeppe | dimitern: for the time being, i think you could leave it as it is | 18:03 |
| dimitern | rogpeppe: 3) add a todo to possibly replace all that with a RU.WriteSettings call | 18:03 |
| rogpeppe | dimitern: we won't be able to delete anything, but i don't think that's a big problem for the time being | 18:04 |
| rogpeppe | dimitern: actually, yes, that will work better | 18:04 |
| dimitern | rogpeppe: ok, let's continue on monday then | 18:04 |
| rogpeppe | dimitern: then deletions will actually work, and we can be more efficient later | 18:04 |
| rogpeppe | dimitern: cool, thanks for going along with me | 18:04 |
| dimitern | rogpeppe: no worries, it was useful | 18:05 |
| dimitern | rogpeppe: have a good weekend! :) | 18:05 |
| rogpeppe | dimitern: i will - going to a folk festival, yay! | 18:05 |
| rogpeppe | dimitern: and you | 18:05 |
| rogpeppe | dimitern: ttfn | 18:05 |
| rogpeppe | g'night all | 18:06 |
| dimitern | yay :) | 18:06 |
| dimitern | fiddles then | 18:06 |
| rogpeppe | oooh yes | 18:06 |
| * dimitern @ eod as well | 18:10 | |
| dimitern | see y'all on monday! | 18:11 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!