[00:00] <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:57] <thumper> wallyworld_: aren't you off today?
[00:57] <wallyworld_> thumper: yeah, about to go to in
[01:00] <axw> thumper: if someone "not lgtm"s something, I need that same person to lgtm to go ahead - is that right?
[01:01] <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:02] <davecheney> axw: depends, could you take them in a straight fight ?
[01:02] <axw> heh :)
[01:11] <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:12] <axw> thumper: thanks
[01:16] <axw> thumper: that LC_ALL thing is existing. do you want me to update those things as I go?
[01:17] <thumper> :) do you feel like making it more up to date?
[01:17] <axw> ... yes
[01:17] <axw> :)
[01:22] <axw> I think I'll write a tool to check import groups
[01:22] <axw> prevent myself from submitting crap
[01:26] <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:34] <thumper> axw: That bug ^^^ seems pretty trivial to do right?
[01:34]  * axw looks
[01:35] <axw> thumper: yes should be pretty simple
[01:37] <axw> I'll take a look in a moment
[01:39] <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:40] <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:41] <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
[02:13] <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:14] <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:16]  * thumper recalls the need to write an email about juju containers
[02:48] <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:56] <thumper> davecheney: if you put an importance on the bug, please mark it triaged
[02:57] <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
[03:00] <axw> thumper: thanks, I've updated checkers -> jc. will merge
[03:33] <thumper> axw: ack
[03:50] <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:51] <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:52] <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 :)
[04:36] <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>
[05:39] <davecheney> relation question
[05:39] <davecheney> can you run the relation-get hook command during the relation-departed hook ?
[06:12] <rogpeppe> mornin' all
[06:22] <axw> morning rogpeppe
[06:22] <rogpeppe> axw: hiya
[07:24] <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:37] <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:40] <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:42] <dimitern> noodles775: I don't think increasing the timeout fixes anything
[07:42] <dimitern> noodles775: it's still fragile
[07:43] <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:44] <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:45] <dimitern> noodles775: ok, let me rephrase - timeout by itself is not enough to fix it, you'll need the above things as well
[07:47] <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:48] <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:49] <noodles775> Right - but it continues to not connect. Let me paste you output, it may make more sense.
[07:58] <noodles775> dimitern: http://paste.ubuntu.com/5991914/
[08:00] <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:01] <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:02] <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:03] <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:13] <rogpeppe> dimitern: reviewed
[08:15] <dimitern> rogpeppe: thanks
[08:15] <dimitern> fwereade: ping
[08:17] <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:18] <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:21] <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:23] <dimitern> rogpeppe: did you have time to check the charm store for possible relation names?
[08:24] <rogpeppe> dimitern: no, i'm going to do that this morning
[08:24] <dimitern> rogpeppe: great
[08:25] <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:27] <rogpeppe> rvba: LGTM
[08:27] <rvba> rogpeppe: ta
[08:34] <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:35] <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:42] <dimitern> rogpeppe: yeah, I'm already half way through
[08:42] <rogpeppe> dimitern: yeah, i saw that - very helpful, thanks!
[08:44] <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:46] <rvba> rogpeppe: sure, I'll have a look.
[08:51] <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:52] <rogpeppe> dimitern: "needed for sync tools" is whether an attribute is needed when pushing to the private storage
[08:53] <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:54] <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
[09:19] <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:20] <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:36] <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:37] <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:38] <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:39] <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:40] <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:41] <fwereade> rogpeppe, oh, nice spreadsheet
[09:41] <rogpeppe> fwereade: so would the ec2 region be part of an account?
[09:42] <fwereade> rogpeppe, might be, if someone almost always wants to use (say) eu-west-1
[09:43] <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:44] <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:45] <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:46] <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:47] <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:48] <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:49] <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:50] <rogpeppe> fwereade: it feels a bit like the responsibilities are mixed up there
[09:51] <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:52] <rogpeppe> fwereade: i'm no longer sure how many levels of inheritance we're aiming for here
[09:54] <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:56] <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:57] <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:58] <rogpeppe> fwereade: hmm, i'd thought the env ended up with all the attributes from the account
[09:59] <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
[10:00] <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:01] <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:02]  * 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:33] <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:34] <dimitern> rogpeppe: it happens to me sometimes since I upgraded to raring - all due to video drivers
[10:35] <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:36] <dimitern> rogpeppe: to get the provider type and from then the private address
[10:36] <dimitern> rogpeppe: and public
[10:37] <rogpeppe> dimitern: i think you should just have a ProviderType API call
[10:37] <dimitern> rogpeppe: ok
[11:13] <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:14] <rogpeppe> dimitern: looks like we can be quite restrictive without it being a problem
[11:15] <dimitern> rogpeppe: so it looks like ([a-z-]+)+ should be enough?
[11:16] <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:17] <dimitern> rogpeppe: sgtm
[11:18] <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:20] <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:21] <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:33] <mgz> standup guys?
[11:59] <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
[12:07] <mgz> geme_: you need to ssh into the state server machine and look at the juju logs
[12:08] <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:10] <mgz> there's also a mongo log that I remember looking at...
[12:13] <mgz> gah, I remember talking to jam about it, but can't find it in the chat logs anywhere
[12:18] <mgz> geme_: any luck? the first few lines of /var/log/juju/machine-0.log should be about mongo state connection
[12:20] <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:21] <geme_> cloud-init.log and cloud-init-output.log ?
[12:22] <mgz> -output please
[12:26] <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:27] <mgz> what do you get if you sudo run thant command from the box now?
[12:33] <geme_> mgz, http://paste.ubuntu.com/5992536/
[12:34] <mgz> oh god, that statecert logging
[12:35] <mgz> geme_: okay, so it installs the wrong version of mongodb (2.0.4-1ubuntu2.1) because the repo add failed
[12:36] <geme_> adding the repo works from the cmdline
[12:37] <geme_> though it needs a return response
[12:42] <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:43] <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:45] <mgz> one thing you could try is reproducing the error with `nova boot --user-data CLOUDCONFIGFILE`
[12:46] <geme_> mgz, when cloud-init runs can it access envars like HTTP_PROXY ?
[12:47] <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:48] <geme_> mgz, /etc/bash.bashrc
[12:49] <mgz> doubt very much cloud-init will pick up that, except perhaps as a side-effect to some subcommands
[12:51] <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:54] <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:55] <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:57] <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:59] <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
[13:00] <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:01] <geme_> mgr, ok I'll ask. Thanks
[13:02] <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:03] <dimitern> rogpeppe: we should have some repo for such useful tools
[13:03] <rogpeppe> dimitern: yeah, they might well be useful to others
[13:05] <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:06] <dimitern> rogpeppe: why did you resolve all my comments on that document?
[13:07] <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:08] <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:09] <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:10] <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:11] <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:12] <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:13] <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:14] <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:18] <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:19] <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:20] <dimitern> rogpeppe: I can make RemoteUnit to be optional
[13:20] <rogpeppe> dimitern: hmm, can't you just omit LocalUnit?
[13:21] <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:22] <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:23] <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:24] <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:28] <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:38] <rogpeppe> dimitern: when you've a moment, try running "go get launchpad.net/juju-utils/cmd/getallcharms"
[13:39] <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:40] <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:41] <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:42] <dimitern> rogpeppe: done, 361M fetched
[13:42] <rogpeppe> dimitern: cool
[13:42] <dimitern> rogpeppe: and charmmeta ?
[13:43] <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:44] <rogpeppe> dimitern: no - it probably should recurse looking for metadata.yaml files
[13:45] <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:46] <dimitern> rogpeppe: charmmeta '..template..' `cat ~/allcharms/urls.txt` fails
[13:46] <dimitern> rogpeppe: it reports Dir==nil on each line
[13:47] <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:49] <dimitern> man.. testing Enter and LeaveScope is a bitch
[13:49] <dimitern> I have to use a scope watcher..
[14:04] <rogpeppe> dimitern: charmmeta is now recursive. couldn't quite resist doing it now :-)
[14:07] <dimitern> rogpeppe: cool, will test it
[14:11]  * rogpeppe must get some lunch now
[15:07] <rogpeppe> trivial review, anyone? https://codereview.appspot.com/13053043
[15:08] <rogpeppe> just some package renaming, mechanical changes
[15:15] <natefinch> rogpeppe: I can review it
[15:15] <rogpeppe> natefinch: thanks
[15:23] <natefinch> rogpeppe: reviewed. LGTM, bonus points for fixing a few of the import sorts
[15:23] <rogpeppe> natefinch: good catch
[15:30] <rogpeppe> dimitern, mgz, rvba, jtv, fwereade: another review appreciated please. trying to avoid a prereq. https://codereview.appspot.com/13053043
[15:33] <dimitern> rogpeppe: looking
[15:33] <rogpeppe> dimitern: ta
[15:33] <rogpeppe> dimitern: i okayed this with fwereade previously, BTW
[15:34] <dimitern> rogpeppe: while I'm at it, would you look at https://codereview.appspot.com/13055043
[15:34] <rogpeppe> dimitern: looking
[15:36] <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:37] <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:38] <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:39] <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:40] <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:41] <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:42] <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:43] <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:44] <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:45] <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:46] <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:47] <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:48] <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:49] <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:50] <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:51] <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:52] <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:53] <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:54] <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:55] <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:56] <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:57] <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:58] <dimitern> rogpeppe: I don't quite agree the burden is that big
[15:59] <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
[16:00] <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:01] <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:02] <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:03] <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:04] <rogpeppe> dimitern: otherwise we'd have one project for each package
[16:05] <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:06] <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:07] <dimitern> rogpeppe: it depends on how easy we make it
[16:08] <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:09] <dimitern> rogpeppe: reviewed
[16:10] <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:11] <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:12] <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:13] <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:15] <dimitern> natefinch: nah.. c'mon you can't agree like that :)
[16:16] <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:18] <natefinch> dimitern: sure
[16:19] <dimitern> natefinch: ta
[16:23] <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:24] <natefinch> dimitern: ahh, ok
[16:25] <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:51] <dimitern> natefinch: thanks
[16:52] <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:56] <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:57] <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:58] <rogpeppe> dimitern: i'm looking now
[16:59] <dimitern> rogpeppe: cheers
[17:03] <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:04] <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:05] <rogpeppe> natefinch: gofmt isn't far off being able to do it
[17:06] <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:07] <rogpeppe> natefinch: gosym resolves symbols but it doesn't cope with import to . well
[17:09] <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:14] <rogpeppe> dimitern: can settings attributes be deleted?
[17:15] <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:16] <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:17] <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:18] <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:19] <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:20] <rogpeppe> dimitern: it might be reasonable to say that setting a value to nil will be treated as a deletion
[17:21] <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:22] <dimitern> rogpeppe: so you're saying the operation has to be more sophisticated
[17:23] <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:24] <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:25] <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:26] <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:27] <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:28] <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:29] <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:30] <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:31] <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:32] <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:33] <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:34] <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:35] <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:36] <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:37] <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:38] <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:39] <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:40] <rogpeppe> dimitern: you'd only need one map, of course.
[17:40] <dimitern> rogpeppe: yeah
[17:41] <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:42] <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:43] <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:44] <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:45] <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:46] <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:47] <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:48] <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:49] <dimitern> rogpeppe: yeah, and how is my approach different from what's already happening?
[17:50] <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:51] <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:52] <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:53] <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:54] <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:55] <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:56] <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:57] <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:58] <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:59] <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
[18:00] <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:01] <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:02] <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:03] <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:04] <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:05] <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:06] <rogpeppe> g'night all
[18:06] <dimitern> yay :)
[18:06] <dimitern> fiddles then
[18:06] <rogpeppe> oooh yes
[18:10]  * dimitern @ eod as well
[18:11] <dimitern> see y'all on monday!