/srv/irclogs.ubuntu.com/2013/08/13/#juju-dev.txt

marcoceppiWhere is the cloud-init file that juju pushes to machines placed?00:56
=== flaviamissi_ is now known as flaviamissi
axwmarcoceppi: for lxc, /var/lib/juju/containers/<container>/cloud-init01:10
marcoceppiaxw: I found them in /var/lib/cloud/seed/nocloud-net, let me check that path too01:10
axwmarcoceppi: sorry, I'm looking on the host here01:11
marcoceppiaxw: ack, np. Thank for the info!01:12
axwnps01:12
=== jam1 is now known as jam
davecheneycan anyone help with a tmux session ?06:07
davecheneyquestion06:08
davecheneyusing tera term06:09
davecheneythe tmux bottom line causes the terminal to scroll one line every second06:09
davecheneyany suggestions ?06:09
jamdavecheney: futz with $COLUMNS ?06:10
davecheneyjam: inside the debug-hook ?06:11
jamdavecheney: I don't really know, I would have said trying it in the container before starting the tmux session. It sounds like tera disagrees with 'write to the end of the line' as off-by-one06:13
davecheneyjam: yeah06:13
jamwhich is why some people say you should code to line 79 because sometimes the terminal wraps the final \n. :)06:13
davecheneyi don'06:13
davecheneyi don't know if its fixable right now06:13
davecheneyi might have to twiddle the tmux conf06:13
jamdavecheney: can you just use xterm, etc ?06:13
davecheneyjam: we are using xterm06:14
davecheneywell, tera says it's xterm06:14
jam(10:09:03) davecheney: using tera term06:14
jamdavecheney: right, tera is lying and claiming its xterm, but then treating screen width differently (from what you've said at least).06:14
davecheneyi've tried vt10006:15
davecheneyvt3206:15
davecheneyvt5206:15
davecheneyetc06:15
jamhave you tried a terminal other than tera ?06:15
davecheneyjam: not an option sorry, we're in an SOE06:15
davecheneystandrard operating environment06:15
davecheneywhich lacks freedom06:15
davecheneyjam: tmux thinks the window is one character larger than it claims06:18
davecheneywider06:18
jamdavecheney: my guess is that tmux writes " "*39 + "\r", and tera thinks that the "\r" needs to wrap.06:20
jamI haven't found ways to tweak either tera or tmux in my googling.06:20
davecheneyjam: thanks06:20
davecheneylet me try one thing06:21
jamWindows shells tend to do the same thing wrong (which is why in our bzr code, we always write to COLUMNS-1, though some terminals let you write all the way to COLUMNS)06:21
davecheneyjam: any idea where the default ubuntu branded tmux conf lives ?06:23
jamdavecheney: http://askubuntu.com/questions/192401/where-is-the-default-tmux-conf-file-located06:23
jamsays there isn't one06:24
jambut there are examples06:24
jamin /usr/share/doc/tmux/examples06:24
=== tasdomas_afk is now known as tasdomas
davecheneyjam: where does debug hooks get it's magic one from ?06:27
davecheneyis this byobu ?06:27
davecheneyah, it is06:28
axwdavecheney: it'll use ~/.tmux.conf if it exists06:30
axwon first execution, that'll get populated with "source /usr/share/byobu/profiles/tmux"06:31
axwsource-file..06:31
davecheneyfuck06:35
rogpeppemornin' all08:07
rvbajam: mgz:  Hi… would any of you be available to update gwacl on the landing bot's machine?08:45
rogpeppefwereade: yo!08:46
fwereaderogpeppe, heyhey08:46
rogpeppefwereade: how's tricks?08:47
fwereaderogpeppe, not bad, had a lovely peaceful weekend08:47
fwereaderogpeppe, and yourself?08:47
rogpeppefwereade: i had a lovely weekend full of frenetic outdoor activity :-)08:48
rogpeppefwereade: and in general things are ok08:49
wallyworld_jtv: are you working on code to call getImageBaseURLs() on the azure provider?08:49
rogpeppefwereade: current area of discussion is how we might make the bulk API call code a little less... bulky08:50
jtvwallyworld_: not working on anything in the provider ATM, why?08:50
rogpeppefwereade: one possible approach is this: https://codereview.appspot.com/1284504308:50
rogpeppefwereade: natefinch has another thought, which you might have seen (automatic code generation)08:51
jtvwallyworld_: the simplestreams code is already working for Azure, apart from a few limitations (images for any location only available in West US; only daily saucy images actually functional yet)08:52
wallyworld_jtv: i'm doing some refactoring to extrude into a common function the url gathering which augments the base url with provider specific ones (if the provider supports such an operation)08:52
wallyworld_so the azure function can be deleted08:52
jtvwallyworld_: that sounds nice... will it grab both the daily and the released images?08:52
wallyworld_jtv: the current function just returns the DefaultBaseURL - i wasn't going to change that behaviour08:53
wallyworld_how is simplestreams working if nothing calls that function?08:53
wallyworld_for azure08:53
jtvwallyworld_: argh, another piece of dead code then!  We just use the same global variable that that function returns.08:54
fwereaderogpeppe, interesting -- it's one of those situations that makes me weep for the lack of generics really08:55
jtvSometimes I find myself wishing for a tool to warn about dead code in Go.  Some kind of lint checker.08:55
wallyworld_the code should have been calling that function :-)08:55
jtvwallyworld_: feel  free to change it!08:55
wallyworld_ok, i'll work on it, thanks :-)08:55
rogpeppejtv: there might have been something like that added to go vet recently08:55
jtvrogpeppe: thanks!  Worth reading up on then.08:56
rvbawallyworld_: sorry to bother you with that… but do you have access to the landing bot?  I need gwacl to be updated there.08:56
wallyworld_rvba: yes, sure08:56
rvbawallyworld_: thanks a lot.08:56
rogpeppefwereade: it's this kind of repetition which was the reason i structured the API the way I did in the first place (so that the magic was all in one place, in the rpc package)08:57
jtvrogpeppe: not seeing anything in the godoc for "go vet."  There is some stuff that might be nice to make more routinely accessible though.08:57
rogpeppejtv: on tip?08:57
jtvThe PPA.08:58
wallyworld_rvba: now on rev 217. correct?08:58
rogpeppejtv: ah, it hasn't even landed on tip yet: https://codereview.appspot.com/12507043/08:58
rvbawallyworld_: perfect, thanks again.08:58
wallyworld_np, anytime08:59
fwereaderogpeppe, yeah, I can see that limiting the spread of magic is generally a good thing09:00
rogpeppejtv: it only works with unexported identifiers though, so i'm not sure if it would flag your case above (what's the dead code you're referring to?)09:01
dimiternfwereade, rogpeppe: fwiw we agreed yesterday to continue with the current approach until we have the uniter talking to the API first, and then proceed to refactor the server-side stuff - as we agree to do it - code generation or reflection09:02
jtvrogpeppe: unused unexported method.  But if it's not landed yet, my individual case is sort of moot.  :)09:03
fwereadedimitern, that sgtm, thanks09:03
fwereadedimitern, I'm kinda suspicious of both approaches ;p09:03
rogpeppejtv: you could always apply the CL locally09:04
dimiternfwereade: I have my doubts as well, but the most important thing for me now is not to lose momentum with the implementation09:04
fwereadedimitern, +10009:04
jtvrogpeppe: to detect a dead method we've already detected?  Thanks.  :-)09:04
rogpeppejtv: lol09:04
rogpeppejtv: but what about all the others we haven't? :-)09:05
jtvTheir time will come.09:05
fwereaderogpeppe, does https://bugs.launchpad.net/juju-core/+bug/1205286 ring any bells?09:39
_mup_Bug #1205286: charm directory permissions now more restrictive <juju-core:New> <postgresql (Juju Charms Collection):Triaged> <postgresql-psql (Juju Charms Collection):Triaged> <https://launchpad.net/bugs/1205286>09:39
rogpeppefwereade: not really, but i regularly campaign against the gratuitous use of non-0777 permissions when creating directories, usually to no avail09:41
noodles775fwereade: I've updated the Makefile to add stable (now that the mongodb deps are there). If you can mark the MP to approved it should land: https://code.launchpad.net/~michael.nelson/juju-core/update-readme/+merge/17916509:42
fwereadenoodles775, thanks, will do09:43
noodles775ta09:43
rogpeppefwereade: hmm, a naive grep doesn't show that as a problem here though.09:43
fwereadenoodles775, done09:43
fwereaderogpeppe, yeah, I was just poking at it and wondering what the deal was09:44
* rogpeppe should really try out the local provider :-)09:45
fwereaderogpeppe, that said, my instincts are leaning towards the "everything should keep working if everything jujey is removed"09:45
fwereaderogpeppe, which would imply the "feature" interpretation09:46
rogpeppefwereade: by "everything", you mean the charm?09:46
fwereaderogpeppe, yeah09:46
rogpeppefwereade: i think it's important that the charm directory be definitively available09:46
rogpeppefwereade: apart from anything, lots of charms use it to put config file templates in etc09:47
fwereaderogpeppe, templates, fine... they'll only be used by hooks, surely09:47
fwereaderogpeppe, but I'm pretty sure that the charm dir should be considered off-limits to anything not running as a hook09:48
rogpeppefwereade: hmm, it's certainly arguable09:48
fwereaderogpeppe, because nothing else has any reason to believe that arbitrary changes are not happening at any given time09:48
rogpeppefwereade: yeah - assuming we overwrite charm dirs rather than duplicate them09:49
fwereaderogpeppe, I think all it takes is assuming that hooks might do anything to the charm dir, and that v1 of a charm can't know what subsequent versions may choose to do instead09:50
rogpeppefwereade: but i'd like to at least know why we're seeing this behaviour - it doesn't look as if we're deliberately using 700 perms09:50
dimiternrogpeppe, fwereade: https://codereview.appspot.com/12849043 - service-related uniter API calls09:50
fwereaderogpeppe, I agree there09:50
rogpeppefwereade: personally i think the charm directory should be considered immutable09:51
rogpeppefwereade: but i realise that's probably a lost battle now09:51
fwereaderogpeppe, one day we'll need some charm format changes, we can force it then, I hope09:52
fwereaderogpeppe, all it really takes is *providing* a directory that charms are expected to write to09:53
fwereaderogpeppe, and then rolling it up with some more changes that are compelling enough to make people want to switch ;p09:53
rogpeppefwereade: tbh i don't think we'll ever do it - we'd be giving up the main value of juju, which is the large base of charms09:53
rogpeppefwereade: yeah, we should definitely provide a charm scratch dir09:54
rogpeppefwereade: we could do that now actually09:54
fwereaderogpeppe, yeah, that sounds smart actually09:54
fwereaderogpeppe, then it's just a matter of communicating it to the charmers, and phasing in requiring its use09:55
rogpeppefwereade: yeah09:55
fwereaderogpeppe, hard to check for automatically but we can certainly put it on the checklist09:55
fwereaderogpeppe, on the other hand, we do lose the rollbackability09:55
fwereaderogpeppe, however I'm not sure anyone's really using that09:55
fwereaderogpeppe, ...but we don't know09:56
rogpeppefwereade: rollbackability assumes that *all* state is in the charm dir09:56
* rogpeppe likes that word09:56
fwereaderogpeppe, true09:56
fwereaderogpeppe, or at least it assumes that external state is regenerated from the contents of the charm dir, but yeah09:57
rogpeppefwereade: anyway, we could theoretically roll back the contents of the scratch dir09:58
dimiternmgz, jam: https://codereview.appspot.com/12849043/09:58
fwereaderogpeppe, yeah09:58
rogpeppefwereade: (assuming there are only files in there - what do we do if people are putting unix-domain sockets in there? :-))09:59
rogpeppedimitern: i'm looking09:59
dimiternrogpeppe: thanks09:59
fwereaderogpeppe, hunt them for sport? ;p09:59
rogpeppefwereade: lol10:00
jamfwereade: so did you have a response for bug #1205286 so that stub can get unblocked?10:05
_mup_Bug #1205286: charm directory permissions now more restrictive <juju-core:New> <postgresql (Juju Charms Collection):Triaged> <postgresql-psql (Juju Charms Collection):Triaged> <https://launchpad.net/bugs/1205286>10:05
jamfwereade: also, you never commented on Placement Directives and I wanted to send that to the list: https://docs.google.com/a/canonical.com/document/d/1Gq8RKyI4uLSYeamz7C2F0tgXdHhTj-d8thKhyX6ae4c/edit?usp=sharing10:10
fwereadejam, I'm still thinking/trying to figure out what's happened with the bug10:10
jamk10:11
jamI don't have a particular stake in the matter, just care that stub has a way forward10:11
jamand no particular rush on placement directives, but sort of a "lets get docs from the sprint out to everyone lese"10:12
fwereadejam, I've just made the only comment I think I want to on placement directives10:19
jamfwereade: well, if you want to barf on the overlaps, you'd need a way to disable the overlap for the current request (i think)10:20
jamand the MaaS case for tags that might be service level, but might be unit level10:21
jamare a bit less obviously "broken" than zone10:21
rogpeppedimitern: reviewed10:25
dimiternrogpeppe: cheers10:25
dimiternrogpeppe: LifeGetter works on units, how can it work on services as well?10:26
rogpeppedimitern: with no problem?10:27
dimiternrogpeppe: with different entities?10:27
dimiternrogpeppe: and we need both Watch for units and services10:27
rogpeppedimitern: LifeGetter works on any entity that has a Life method10:27
dimiternrogpeppe: we need different methods for watching units and services10:28
rogpeppedimitern: all you'd need to do is make the auth func return ok for both units and services10:28
rogpeppedimitern: really?10:28
dimiternrogpeppe: yes10:28
rogpeppedimitern: don't we watch both units and services with a Watch method?10:28
rogpeppedimitern: which has the same signature for each10:28
dimiternrogpeppe: why should the same method operate differently?10:28
dimiternrogpeppe: that's mixing responsibilities10:29
rogpeppedimitern: it's not operating differently - we're passing in entity names to watch, and it's doing that10:29
dimiternrogpeppe: it should work either on units or on services, you can't mix them10:29
rogpeppedimitern: why not?10:29
rogpeppedimitern: we're saying "watch these things"10:30
rogpeppedimitern: where a thing is uniquely identified by its tag10:30
dimiternrogpeppe: how about configsettings then?10:30
dimiternrogpeppe: by your logic we shouldn't need another method10:30
rogpeppedimitern: we still need an entry point for that10:30
rogpeppedimitern: because we don't have an entity for config settings10:31
rogpeppedimitern: we don't consider having a Lifer interface to be mixing responsibilities10:31
dimiternrogpeppe: so you propose to create a different authFunc just for LifeGetter and Watch works on either only units or only services10:31
dimiternrogpeppe: I don't think it should accept mixed units and services tags in one call10:32
rogpeppedimitern: why not?10:32
rogpeppedimitern: it's a bulk call10:32
dimiternrogpeppe: because it complicates the tests - we need to test all variations10:32
rogpeppedimitern: it takes a load of entities10:32
rogpeppedimitern: it's no more complicated than having extra entry points10:33
rogpeppedimitern: probably less so10:33
rogpeppedimitern: and the API calls make more sense that way, i think10:33
dimiternrogpeppe: no, the simplification for having a single entry point will be transfered to increasing the test size 4-fold10:34
rogpeppedimitern: why should the API provide both UnitLife and ServiceLife when a single Life entry point is sufficient?10:34
rogpeppedimitern: really? isn't it a single extra entry in the argument slice?10:34
dimiternrogpeppe: and we're talking about both Watch and Life - so AgentEntityWatcher and LifeGetter both need to work on units and/or services10:35
rogpeppedimitern: isn't that trivial? we just make an auth function that allows both the unit or the unit's service10:35
dimiternrogpeppe: no, each tests tests at least 3 cases: valid tag, invalid tag, valid but unauthorized tag10:35
rogpeppedimitern: so that's 2 more entries in the arg slice, no?10:36
dimiternrogpeppe: when we take into account services and units can be passed, that comes to 6 cases10:36
dimiternrogpeppe: and in addition, we need to add a case when the tag is neither service nor unit10:37
dimiternrogpeppe: so that's 7 cases to test10:37
dimiternrogpeppe: i agree they can be done in a single call10:37
dimiternrogpeppe: just the testing logic will be a bit more complicated after we get the results and check the resources10:38
rogpeppedimitern: yes, you'll have some extra cases to check, but you'd need to check all those cases if you had a different API call, no?10:39
dimiternrogpeppe: and when relations come into play, I suppose their Life will be using the same entry point, so that's 3 more cases10:39
rogpeppedimitern: sure - but isn't it great that we can use the same API call (and code) for getting the Life of any entity?10:39
rogpeppedimitern: doesn't that make it particularly nice that we are actually sending tags over the wire, rather than just ids?10:40
dimiternrogpeppe: I'll think about it a bit10:40
rogpeppedimitern: we already have the LifeGetter implementation, which can be used for all of this10:40
jamrogpeppe: so I think they may end up in different facades, and the different facade may need to restrict what they are allowed to look at. But as long as the Auth is valid (unit agents can't look at machines, for example) having a unified call sounds good.10:41
rogpeppedimitern: (which, FWIW, would be wrong to use if you continue in the same direction - why should we have Life and ServiceLife not UnitLife and ServiceLife?)10:41
rogpeppejam: yeah, LifeGetter allows arbitrary restrictions on what you can look at10:42
dimiternrogpeppe: so the auth func for LifeGetter should check AuthOwner for units and 1) get auth'ed entity, 2) compare it's service tag with the given tag - for services10:43
dimiternrogpeppe: and basically the same for AgentEntityWatcher as well10:44
rogpeppedimitern: well, it could be simpler than that10:44
rogpeppedimitern: it sounds reasonable. i'll wait to see the code10:45
rogpeppedimitern: it basically comes down to: return tag == unit.Tag() || tag == names.ServiceTag(unit.ServiceName)10:46
jamrogpeppe: what is '.tsv' stand for? tab-separated-value (like csv, just with a \t ?)10:48
dimiternrogpeppe: I though you suggested having a way to get the auth'ed entity from the authorizier10:48
rogpeppejam: yeah10:48
dimiternrogpeppe: like GetAuthEntity() interface{}10:48
rogpeppedimitern: yes - the above is assuming you've got unit from there10:48
rogpeppejam: i thought it was better than .txt and it does seem to have precedent10:48
davecheneygood evening gentlemen10:49
davecheneyi have a request from the far east10:50
davecheneyi need to build a version of juju that does not attempt to create security groups on openstack/hp cloud10:50
davecheneyis anyone able to help me ?10:51
rogpeppedavecheney: do you need tests to pass too?10:51
davecheneyrogpeppe: no10:52
rogpeppedavecheney: have you tried just commenting out the code that creates the security groups?10:52
davecheneyrogpeppe: yes, that is what I wanted to do10:52
davecheneyi was hoping for some advice10:53
davecheneymaybe I should just turn off the firewaller10:53
rogpeppedavecheney: hmm, without security groups, how can you open any ports at all?10:54
rogpeppedavecheney: 'cos presumably by default all ports are closed10:54
davecheneyrogpeppe: according to marcoceppi they don't actually firewall anything10:55
davecheneyjust prevent most people from being able to deploy more than 10 services in total on their hp cloud account10:55
jamrogpeppe: I know in ec2, the default is that all machines in the same security group can talk to eachother on any port. I'm not sure about hp/openstack in general.10:55
rogpeppedavecheney: ah, so it should be irrelevant10:55
rogpeppejam: presumably these machines won't be in the same security group (because we can't create one)10:55
davecheneyjam: yeah, security groups in ec2 work10:56
davecheneythey don't in hp cloud10:56
jamrogpeppe: I don't know that dave is asking for 0, just not 1-per-machine.10:56
davecheneyso the creation of the groups just presents an annoying limit10:56
rogpeppedavecheney: ah, so you can create one group for the whole environment?10:57
davecheneyrogpeppe: preferably zero10:57
davecheneygroups10:57
davecheneywallyworld_: poing10:58
davecheneyping10:58
rogpeppedavecheney: in that case, i'd just see what happens when you change environ.setUpGroups to return nil, nil10:58
davecheneyrogpeppe: ok, will do10:58
wallyworld_davecheney: wat10:58
fwereadedavecheney, otherwise firewall-mode global will create just one, if that helps?10:59
marcoceppiwallyworld_: we're going to be demoing on az1 and az2 "tomorrow", however last I checked az2 and az3 don't have simple stream data10:59
rogpeppefwereade, davecheney: yes, that was going to be my other suggestion10:59
wallyworld_marcoceppi: maybe not, i haven't checked. i did some initial data for az1 using the juju tool. if we need it for the other regions it will need to be hand crafted. when is "tomorrow"?11:00
davecheneyfwereade: that might be better11:01
jammarcoceppi: there won't be simplestreams data in cloud-images, since hp isn't (yet) a CPC11:01
davecheneyi'll try that11:01
marcoceppiwallyworld_: 12 hours from now11:01
jamwallyworld_: can we generate stuff and put it with the tools we copied over there ?11:01
wallyworld_marcoceppi: ok, i'll see if i can whip something up tonight. i may not be around tomorrow as it's a holiday here. how long will you e around for now?11:02
marcoceppijam: az1 data exits already, it'd be nice if az2 and 3 were also in there11:02
marcoceppiwallyworld_: I'll be around for 2-3 hours11:02
wallyworld_jam: what he said - the existing nmetadata nwds to be added to11:02
wallyworld_marcoceppi: ok, i'll see what i can do. maybe you could test once i do it11:03
marcoceppiwallyworld_: I'd be happy to test!11:03
wallyworld_ok, leave it with me11:03
wallyworld_marcoceppi: what image ids do we want?11:03
wallyworld_there's a bug i think with those listed11:04
marcoceppiwallyworld_: thanks for taking a look! If not I can dig through the metadata plugin if you run out of time and put it in a seperate bucket11:04
wallyworld_i'll see if i can find it11:04
marcoceppiwallyworld_: I'll double check in the console11:04
marcoceppithey may have changed11:04
wallyworld_ok, if you tell me the images ids for az2 and az3 that would be great11:04
wallyworld_marcoceppi: what series also11:04
wallyworld_precise?11:04
jamwallyworld_: I see endpoint data but no content: https://cloud-images.ubuntu.com/releases/streams/v1/com.ubuntu.cloud:released:hpcloud.sjson11:05
wallyworld_jam: the data isn't there yet. it's in the hp cloud public bucket11:05
jamah, ok.11:05
* wallyworld_ wishes it was there11:06
davecheneysorry, marcoceppi has lagged out11:06
davecheneyhe'll be back in a sec11:06
davecheneymarcoceppi: firewall-mode: global11:09
davecheneyfwereade: firewall-mode: global might be the trick for us11:09
fwereadedavecheney, cool11:10
fwereadedavecheney, glad it's good for something, I was worried about it for a while ;p11:10
davecheneyfwereade: that means we're down to 2 sec groups per student11:10
davecheneythat'll do11:10
davecheneywe can squeak by11:10
davecheneythanks11:11
marcoceppiwallyworld_: yes, precise11:12
wallyworld_ok11:12
marcoceppiwallyworld_: would it be possible to put the data in the same bucket as the one davecheney sends out during ANN for juju-core testing on hpcloud?11:12
wallyworld_marcoceppi: yeah,that's what i'm doing11:13
marcoceppiwallyworld_: <3 awesome, thanks!11:13
wallyworld_updating existing metadata11:13
wallyworld_just need the images ids :-)11:13
davecheneywallyworld_: thanks man11:13
wallyworld_anytime11:14
davecheneyso far we've been able to avoid spinning custom tools11:14
davecheneythere is one more problem which I'm not sure I can fix without changing the tools11:14
wallyworld_yes?11:14
davecheneybyobu doesn't interact well with tera term11:15
marcoceppiwallyworld_: AZ2: 68481 AZ3: 4985011:15
wallyworld_ta11:15
davecheneyso I need to put `apt-get remove byobu` in the cloud-init so I can remove byobu and fall back to tmux11:15
jamwallyworld_: don't you need the uuids or HP being Diablo just has the short names11:16
jam?11:16
natefinchmorning all11:16
wallyworld_jam: we support both uuids and ints transparently11:17
natefinch(and afternoon and evening as the case may be)11:17
jamhi natefinch11:17
marcoceppiwallyworld_: I created simplestream data a while a go for az3, if it helps11:18
davecheneynatefinch: おはようございます11:19
wallyworld_marcoceppi: i've done the new data, just using the validation tool to smoke test it. but i have no perms to re-upload it turns out11:19
marcoceppiwallyworld_: ack11:20
davecheneywallyworld_: sad trombone11:20
wallyworld_marcoceppi: do you have permission to write to the public bucket?11:20
natefinchdavecheney: ha.  Sorta surprised google knew what to do with that.11:20
marcoceppiwallyworld_: no that I know of11:20
wallyworld_marcoceppi: is arosales around? he can11:21
davecheneywallyworld_: arosales has been up for about a day11:21
davecheneyhe's been sorting out all our shit today11:21
marcoceppiwallyworld_: he may be around, but I'm not sure11:21
davecheneyi think he's turned in11:21
wallyworld_ok11:22
marcoceppislacker ;)11:22
noodles775rogpeppe: RE the HP provider returning ErrNoInstances, I just went to make the change to shortAttempt that you mentioned, but afaics from the bug, it's not related? That said, I *think* I found another potential cause... details in the description: https://codereview.appspot.com/1279504511:22
natefinchonly one day?11:22
wallyworld_so what you can do is create a new container, make it globally readable, and put tools and new metadata in it11:22
marcoceppiwallyworld_: I might be able to actually, let me check11:22
wallyworld_and set the public-bucket-url accordingly11:22
rogpeppenoodles775: looking11:24
wallyworld_marcoceppi: davecheney: i have the new metadata if only we could write it11:25
wallyworld_you guys need to give me more advance notice11:25
davecheneywallyworld_: i'll thank you personally for that helpful comment next week11:26
wallyworld_davecheney: just saying11:26
jamwallyworld_: I have perms to the hp public bucket, but I thought you did, too.11:26
davecheneyyeah, sorry, we're really hulk smashing this whole training11:26
wallyworld_davecheney: just being a prick, sorry11:27
davecheneys'ok11:27
davecheneyif that was the worst thing that had happende this week, i'd be upset11:27
davecheneybut it isn't11:27
davecheneynot by a long shot11:27
wallyworld_jam: i logged in and can only see us-west11:27
wallyworld_and cannot see the public bucket11:28
marcoceppiwallyworld_: I have a bukkit11:28
wallyworld_marcoceppi: we may be able to write the files11:28
rogpeppenoodles775: good catch!11:28
marcoceppi\o/11:28
jamwallyworld_: there is only 1 storage11:28
jamthere are 3 computes11:28
jamthey all use the same storage11:28
rogpeppenoodles775: looks like a good fix. it would be nice to be able to test it though.11:28
jamaz3.geo-1, vs just geo-111:28
jamIIRC11:28
wallyworld_jam: i can only see 3 containers, none of which is the public bucket when i log in11:29
jamwallyworld_: I see roughly the same thing, but we have the tools somewhere in that account :)11:29
jamwallyworld_: "control-bucket: juju-dist"11:30
wallyworld_jam: ah, maybe i am a dickhead, there is a drop down with projects which i didnt see11:30
wallyworld_let me try selecting the right project11:30
jamwallyworld_: I didn't see it either11:30
jamwallyworld_: https://console.hpcloud.com/object_store/region-a_geo-1/containers/juju-dist11:30
wallyworld_yep, uploading now11:31
wallyworld_marcoceppi: ready for testing11:31
wallyworld_davecheney: ^^^11:31
jamdimitern, rogpeppe, standup? https://plus.google.com/hangouts/_/f497381ca4d154890227b3b35a85a985b894b47111:33
davecheneywallyworld_: ok, will test11:33
davecheneywallyworld_: thank you11:33
jamaxw: ^^11:33
wallyworld_davecheney: np at all.  if it doesn't work, i'll fix, let me know11:33
davecheneywallyworld_: http://paste.ubuntu.com/5980804/11:35
davecheney^ it's copying tools, is this expected ?11:35
jamdavecheney: it wasn't expected for me11:35
jamdo you have public-bucket-url set for HP?11:35
davecheneyjam: sorr, i might have screwed up11:36
wallyworld_yes, looks like public bucket is not set right if it can't see tools11:36
davecheney    public-bucket-url: https://console.hpcloud.com/object_store/region-a_geo-1/containers/juju-dist11:36
wallyworld_davecheney: https://region-a.geo-1.objects.hpcloudsvc.com/v1/6050252975391011:36
davecheneyok, that was what I had before11:37
davecheneymy bad11:37
wallyworld_davecheney: soon, public bucket will not be necessary at all \o/11:37
davecheney2013-08-13 11:37:34 INFO juju provider.go:734 environs/openstack: started instance "1670895"11:37
davecheney2013-08-13 11:37:35 INFO juju supercommand.go:276 command finished11:37
davecheneygot an instance in az311:37
davecheneyrocking11:37
wallyworld_yay \o/11:37
marcoceppiditto over here for az211:38
* wallyworld_ is happy he wrote the image validation tool to check the metadata prior to copying to the bucket11:38
marcoceppijust waiting for juju status to come back11:38
davecheneythanks so much for your help11:39
wallyworld_my pleasre. good luck with the demo :-)11:39
marcoceppiwallyworld_: def, thank you sir!11:40
wallyworld_anytime11:40
noodles775rogpeppe: Thanks! And yeah - that was my next question. Any relevant examples you could point me at for tests? The ones in the respective provider_test.go are just for one particular function. Otherwise I'll hunt around.11:42
rogpeppenoodles775: in a call. be with you in a bit.11:42
mgzdavecheney: I'm going to poke the release tarball script to use the revs from our new dependencies.tsv file11:47
davecheneymgz: sounds good to me11:53
rvbaAnyone up for a one-liner? https://codereview.appspot.com/1285204311:53
davecheneyrvba: worst pickup line, ever11:54
jamwallyworld_: did you see: https://bugs.launchpad.net/juju-core/+bug/121114711:54
_mup_Bug #1211147: Deploying service to bootstrap node causes debug-log to spew messages <juju-core:Triaged> <https://launchpad.net/bugs/1211147>11:54
jamYou might want to give it a poke, since you have the most rsyslogd knowledge on the team.11:54
davecheneyjam: yeah, jonathon totally blew up his environment with that one11:55
wallyworld_jam: didn't see it, will look11:55
marcoceppifwereade: I've got a question about optional relations in juju, if you set a relation to be optional: False, what happens?11:55
fwereademarcoceppi, nothing at all11:55
davecheneyfwereade: is it purely decorative ?11:56
fwereadedavecheney, essentially yes11:56
marcoceppifwereade: will it ever do anything?11:56
davecheneyfwereade: for ceremonial occasions ?11:56
fwereademarcoceppi, it is not currently a high priority11:56
marcoceppiI must admit, it's really pretty :)11:56
wallyworld_jam: do we even support putting a service on the bootstrap node?11:57
marcoceppifwereade: what's the idea of what it would do in the future when you guys are done with priority things and bored and want to implement it?11:57
davecheneywallyworld_: sure, didn't you write that ?11:58
davecheneyor are you asking, as an optoin we'd be proud to tell customers about ?11:58
wallyworld_i didn't write it11:58
wallyworld_i really didn't think we supported it11:58
fwereadedavecheney, marcoceppi: there was talk, a long time ago, that we might automatically create non-optional relations between matching services, but that always seemed kinda crazy to me12:00
fwereadedavecheney, marcoceppi: there *may* be some mileage in the idea that we just flag non-optional relations that are not satisfied12:00
fwereadedavecheney, marcoceppi: but doing anything automatic in response seems like a really bad idea to me, because even if we think of something useful to do with it I can't see how it wouldn't be a terrifying behaviour change12:01
marcoceppifwereade: My thought was, if a relation is not optional, it wouldn't "deploy" the charm until juju was aware of the relation via add-relation, must like the subordinate mechanism12:01
davecheneyfwereade: some background, our students are interesting optional: false relations because they are thinking it will help them know when a service is properly deployed12:01
davecheneyie, all it's non optoinal relationships are fulfilled12:02
fwereadewallyworld_, rogpeppe, I can hear you12:04
fwereadewallyworld_, rogpeppe, but can't seem to say anything you can hear... brb12:04
jamfwereade: one more thing for you to glance at quickly: https://code.launchpad.net/~rogpeppe/juju-core/359-no-lax/+merge/178357  (As you were the one that implemented the StartSync vs Sync schism, you should at least be aware that it has all been migrated to Sync)12:16
dimiternrogpeppe: updated https://codereview.appspot.com/1284904312:19
dimiternjam: take a look as well? ^^12:19
rogpeppejam: i'd like your thoughts on that too12:20
rogpeppejam: 359-no-lax that is12:20
wallyworld_fwereade: we lost you again?12:20
fwereadewallyworld_, I can hear you, but I'll rejoin12:21
fwereadewallyworld_, yeah, it doesn't want to let me rejoin :/12:21
axwjam: sorry, was having dinner earlier12:28
jamaxw: np. I don't know that it will work in your Timezone, but we do a whole group get-together at UTC 11:3012:29
jamI think Tim was thinking of doing another one earlier in the day12:29
jamfor those in the high UTC offsets.12:29
jamaxw: I added you to the calendar entry, but it is optional for you, since it is roughly dinner/family/anything-that-isn't-work time for you.12:30
axwjam: thanks for that. I hit "maybe" because of that, but I'll try to make it sometimes. If there's something important, just let me know and I'll be there12:31
jamaxw: it is just our daily "catch up with eachother on what we're working on". You should have a weekly 1:1 with either Mark or Tim already for the "important" things.12:32
dimiternrogpeppe: does it look better now?12:39
rogpeppedimitern: sorry, still in a call12:40
rogpeppefwereade: you've frozen again...12:41
fwereadewallyworld_, rogpeppe, did I get cut off again?12:42
rogpeppefwereade: yes12:42
wallyworld_yep12:42
dimiternrogpeppe: np12:43
dimiternnatefinch, mgz: second review on https://codereview.appspot.com/12849043 please?12:43
mrammhey all12:43
mgzdimitern: sure12:43
dimiternmgz: cheers12:44
dimiternmgz: just note I forgot to update the description - ServiceLife, ServiceCharmURL and WatchService are gone, replaced (as suggested by rog) with Life, Watch amd CharmURL handling both units and services12:45
mrammis it an lxc prereq issue that causes this error:12:46
mramm08:41 danwest: $ juju -v bootstrap12:46
mramm08:41 danwest: 2013-08-13 12:40:06 INFO juju.environs.local environprovider.go:32 opening environment "local"12:46
mramm08:41 danwest: 2013-08-13 12:40:06 ERROR juju.environs.local net.go:18 cannot find network interface "lxcbr0": net: no such interface12:46
mramm08:41 danwest: 2013-08-13 12:40:06 ERROR juju.environs.local environprovider.go:44 failure setting config: net: no such interface12:46
mramm08:41 danwest: 2013-08-13 12:40:06 ERROR juju supercommand.go:235 command failed: net: no such interface12:46
mramm08:41 danwest: error: net: no such interface12:46
dimiternmramm: you need sudo juju bootstrap with the local provider12:47
mrammahh12:47
dimiternmramm: but apart from that it might be something else12:47
dimitern(prereq issue)12:47
mrammok12:48
mrammI will tell him to try that12:48
mrammalso we should give a better error message there too12:48
mrammsudo did not fix it12:49
dimiternso it's a different issue12:49
dimiternwe recently updated the README I think wrt the local provider12:50
dimiternnope, it's not in yet12:50
mrammalso, installing lxc was not the fix12:52
mrammI am having him file a bug12:52
dimiternmramm: I found this thread http://comments.gmane.org/gmane.linux.kernel.containers.lxc.general/3819 - it says to check /var/log/upstart/lxc* for errors12:54
danwestdnsmasq: failed to create listening socket for 10.0.3.1: Address already in use12:55
danwesthmm, if true should handle that a bit more gracefully12:55
danwestlooking into it now12:55
mrammagreed12:56
dimiterndanwest: yup, that's what was mentioned in that thread as well12:56
dimiternmramm, danwest: it seems an lxc issue though, not a juju issue12:56
mrammjuju should handle that if nessisary12:56
mrammI mean we should talk to serge and whatnot, but juju needs to just work or at the very least give good error messages12:57
dimiternmramm: agreed, that specific error means gonet wasn't able to open the interface lxcbr012:58
danwestsame issue after killing dnsmasq12:58
dimiterndanwest: take a look what's listening on 10.0.3.112:58
danwesttftp13:00
danwestnothing13:01
dimiternmaybe it starts dnsmasq twice or something? anything else useful in the log?13:01
danwestnothing in the log, actually the entry must have been old as I moved the log aside and re-ran - no new lxc log13:03
danwestentry was not timestamped13:03
dimiterndanwest: sorry, perhaps hallyn_ or someone else with more in-depth knowledge of lxc can help13:04
danwestdimitern: thx13:06
hallyn_danwest: i need to deal with some movers for the next hour.  addres already in use is weird.  can you reproduce at will?  can you email or pastebin a reproducer?13:08
danwesthallyn_: will try, thx13:09
hallyn_(or just a launchpad bug against lxc :)13:10
hallyn_danwest: oh wait - are you starting containers nested, inside lxc?13:10
danwesthallyn_: nope13:11
hallyn_danwest: if so then you may be running an older lxc - it shoudl fall back to 10.0.4.x if 10.0.3.x is taken by the13:11
hallyn_ok13:11
hallyn_thx, ttyl :)13:11
dimiternjam: so, suggestions about what error should we return when len(entities) == 0 ?13:12
dimiternjam: thanks for the review btw13:12
mgzhm, I find that latest branch a little hard to follow dimitern13:16
dimiternmgz: oh? what's not clear?13:16
mgzvarious changes made so CharmURl also support services? then I'm not clear on the new getCanAccessUnitOrService func13:19
dimiternmgz: yes, as rogpeppe suggested13:19
dimiternmgz: the new authFunc checks if you're allowed to access the given tag, which is either a unit or service13:20
dimiternmgz: in the latter case, you can access it only if it matches your authentication entity (unit)'s service13:20
mgzokay, and it works, because a unit tag will never match a service tag,13:21
dimiternmgz: it's used in CharmURL to allow both unit and service tags as args13:21
mgzso the extra code will always get passed over in the unit case...13:21
dimiternno13:21
dimiternthe code is the same for a unit or a service - both support CharmURL (string, bool)13:22
mgzright, but passing a unit to CharmURL now uses a different auth check13:22
dimiternyes13:22
mgzbut... if a unit is passed, it will still end up calling AuthOwner(tag) the same as before13:22
dimiternno actually, it's still the same13:22
dimiternit's just extended13:22
dimiternyeah13:23
dimiternin case it's not an auth'ed service tag13:23
dimiternAuthOwner will never work for anything else than a unit tag13:23
mgzthose two funcs don't actually need closures, right?13:23
dimiternand services cannot log in anyway13:23
mgzcould you put them at top level with doc comments before assigning into UniterAPI?13:24
dimiternwe need to pass them to LifeGetter and AgentEntityWatcher as well13:24
dimiternwhy? they need the authorizer that was passed to NewUniterAPI13:25
dimiternand they're used only there13:25
dimiternusing closures like this is nice, because we're currying the authorizer as an implict arg13:26
mgzah, they do need closures, hadn't notices authorizer was a param not a package13:26
dimiternyep13:26
mgzwell, lgtmed.13:26
dimiterncheers!13:26
dimiternrogpeppe: I'll wait for you to take a look before landing13:27
rogpeppedimitern: i'm still looking at it13:27
rogpeppedimitern: reviewed13:41
dimiternrogpeppe: thanks13:42
dimiternrogpeppe: i don't want to return Entity there - I want the real thing13:42
dimiternrogpeppe: in GetAuthEntity13:42
rogpeppedimitern: what?13:42
rogpeppedimitern: state.Entity is more real than interface{}13:43
dimiternrogpeppe: but does it have CharmURL and any other method I might need?13:43
rogpeppedimitern: no, but anything is better than bare interface{}13:43
rogpeppedimitern: at least state.Entity suggests the type that it might be returning13:44
dimiternrogpeppe: so what, for each method we need we have to change state.Entity to add that method?13:44
rogpeppedimitern: the set of types13:44
rogpeppedimitern: ???13:44
rogpeppedimitern: interface{} has no methods at all.13:44
dimiternrogpeppe: no, but I can do entity.(*state.Unit).CharmURL() on it13:45
rogpeppedimitern: just as you can if it's state.Entity13:45
rogpeppedimitern: there's no difference in that respect13:45
dimiternrogpeppe: so you can cast any type, no just an interface{} ?13:45
rogpeppedimitern: um, yes?13:45
dimiternrogpeppe: c-style?13:45
rogpeppedimitern: any interface type13:45
rogpeppedimitern: and state.Entity is an interface13:46
dimiternrogpeppe: hmm13:46
dimiternrogpeppe: ok then, I'll try it13:46
rogpeppedimitern: thanks13:46
* rogpeppe gets some lunch13:47
dimiternanyone: next one in line https://codereview.appspot.com/1285004413:55
jamnatefinch: did you see: https://code.launchpad.net/~michael.nelson/juju-core/1208504-post-bootstrap-hp-no-instances-found-try2/+merge/179899 I think noodles775 has touched the same code as you, but I might be thinking of the wrong patch (I've gone through a lot of patches today :)13:58
jamdimitern: len(entities) == 0 can just be kept the same way. If we do decide to change it, then "ErrNoArgs" or something along those lines. But it would be a policy change across the board so it should be thought about and discussed I think.13:59
* jam I'm off for now13:59
noodles775natefinch, jam: happy to reject mine if it's already fixed :)14:00
dimiternjam: I decided to remove it14:03
dimiternjam: when we introduce backward-incompatible changes to params we'll have to take that into account, like for SetStatus14:03
jamdimitern: well we need it in place *before* we make an incompatible change, otherwise the "old servers don't error on new requests" is still valid.14:05
jamanyway, it is a separate change from what you're doing.14:05
dimiternjam: yep14:06
natefinchjam, noodles775:  looking now14:08
natefinchjam, noodles775: looks very different from my changes.  Mine is more focused on when you really haven't bootstrapped yet, rather than bootstrapping not being finished14:14
natefinchjam, noodles775: I think they're complimentary changes, and shouldn't affect one another14:14
noodles775natefinch: Cool - thanks for checking.14:15
jamah, both noodles775 patches. https://codereview.appspot.com/12755043/ vs https://codereview.appspot.com/12795045/14:16
jamnoodles775: is https://codereview.appspot.com/12795045/ supposed to supersede the other?14:17
noodles775jam: yeah it is. I'd not realised you'd commented on the former - it should be closed.14:18
* noodles775 closes.14:18
jamnoodles775: I tend to drive reviews from my email folder, so that doesn't always help :)14:18
dimiternmgz, jam, rogpeppe: when you have some time PTAL https://codereview.appspot.com/1285004414:18
fwereaderogpeppe, so, https://codereview.appspot.com/12352044/14:22
fwereaderogpeppe, your analysis is I think correct14:22
* rogpeppe breathes a sigh of relief14:22
fwereaderogpeppe, but you removed a great big pile of test coverage with the "fix"14:22
dimiternrogpeppe: also updated https://codereview.appspot.com/12849043/14:22
fwereaderogpeppe, and AFAICT it could have been resolved with a LaxStringsWatcher at the end of the unit and machine tests14:23
rogpeppefwereade: really?14:23
fwereadeer LaxNotifyWatcher14:23
rogpeppefwereade: what's no longer tested?14:23
fwereaderogpeppe, well, unless you removed the coalescence behaviour and verified that the tests failed14:23
rogpeppefwereade: i didn't understand why StartSync tests coalescence that Sync doesn't14:23
fwereaderogpeppe, which they wouldn't have done, because the Sync version essentially forces coalescence irrespective of watcher implementation14:24
fwereaderogpeppe, because the Sync tests force all events to have been read before we read from the watcher14:24
fwereaderogpeppe, the StartSync ones were explicitly there to test what happened when we read from the out channel before the in channel was exhausted14:25
rogpeppefwereade: i still don't quite see it14:26
rogpeppefwereade: the difference between StartSync and Sync is just one or two scheduling decisions. I don't see how that can be the basis of a reliable test.14:27
fwereaderogpeppe, the point is that Sync will always pass, while StartSync will sometimes fail if the behaviour's wrong14:27
rogpeppefwereade: perhaps you could describe to me a possible failure mode when using StartSync14:28
fwereaderogpeppe, more reliability is good, I would be happier if the tests could reliably pass/fail in all circumstances14:28
rogpeppefwereade: so I can understand what we're trying to guard against14:28
sidneifwereade: https://codereview.appspot.com/12859043 and https://bugs.launchpad.net/juju-core/+bug/1201503 's comments for some investigation14:30
_mup_Bug #1201503: Add os disk constraint <constraints> <juju-core:Triaged by sidnei> <https://launchpad.net/bugs/1201503>14:30
* arosales reading backscroll, looks like wallyworld_ jam figured out the public bucket issue14:31
fwereadesidnei, cheers14:32
* fwereade continues to try to marshal a clear argument for rogpeppe14:33
* rogpeppe continues to wear a slightly puzzled expression14:34
* fwereade might have come up with an illustrative question14:37
rogpeppefwereade: are you talking specifically about testing the behaviour of the collect function?14:38
fwereaderogpeppe, how is it possible for us to test event coalescence when Sync is the only tool in play?14:38
fwereaderogpeppe, yeah14:38
fwereaderogpeppe, whenever we Sync we force the watcher to finish handling all events before we even try to read from Shanges14:39
fwereaderogpeppe, er Changes14:40
rogpeppefwereade: how's that?14:40
rogpeppeoh, i see14:40
fwereaderogpeppe, so you are perfectly correct about what was going on14:41
fwereaderogpeppe, it's somewhat interesting that a case could be made that the tests were failing due to a hard-to-detect bug in the watcher implementation14:42
fwereaderogpeppe, but changing that stopped us from being able to detect similar bugs that would I think unarguably be bugs14:42
rogpeppefwereade: it depends what semantics you expect from StartSync really14:42
fwereaderogpeppe, as opposed to this behaviour which is, er, arguable14:43
dimiternrogpeppe: should I land https://codereview.appspot.com/12849043/ now? I think I did all you asked14:43
fwereaderogpeppe, I just expect it to return as soon as possible, and for that to leave a moderate chance of detecting collect bugs14:44
dimiternmgz: hey14:44
rogpeppefwereade: i *think* the tests could still break when coalescence is disabled and using Sync14:44
rogpeppedimitern: looking14:44
rogpeppefwereade: in fact, i might try that out14:45
dimiternmgz: https://codereview.appspot.com/12850044 quick review?14:45
fwereaderogpeppe, I think that NotifyWatcher and StringsWatcher are safe from that, and I couldn't repro it just now, but I didn't try very hard14:45
fwereaderogpeppe, any more complex watcher where there's another layer in play is vulnerable14:45
fwereaderogpeppe, but NW and SW only have one select loop, and as soon as the last event's been delivered the watcher finishes processing it before going round again and potentially sending on Changes14:46
rogpeppefwereade: i think a better way of testing coalescence is to make the test start a goroutine to transfer all events into a buffered channel, then call Sync14:47
rogpeppefwereade: as a specific coalescence test14:49
fwereaderogpeppe, yeah, that sounds nice14:49
fwereaderogpeppe, but that sort of test does get rather tricky to follow... iirc you argued against that sort of approach when I first proposed it14:50
rogpeppefwereade: istm that's a better approach than spraying StartSync about the place as if it's testing something specific14:50
fwereaderogpeppe, well, hmm14:51
rogpeppefwereade: i'd have only one test like that for each watcher14:51
fwereaderogpeppe, I think that Sync is very hard to justify14:51
rogpeppefwereade: i think that StartSync is even harder to justify :-)14:51
fwereaderogpeppe, expand please, Sync STM to be much further divorced from anything resembling a real situation14:52
fwereaderogpeppe, you know what14:52
rogpeppefwereade: StartSync gives you no guarantees at all, which makes it hard to write reliable tests14:52
fwereaderogpeppe, fuck both sync and startsync, we should just patch out the watcher period14:52
fwereaderogpeppe, but Sync is completely fake and makes it very easy to write completely useless tests14:53
dimiternfwereade: actually that sgtm14:53
mgzdimitern: looking14:53
dimiternfwereade: we call Sync to make sure the events have arrived, if we patch the watcher period, we can guarantee that14:54
fwereadedimitern, hmm, though, I think we still need *some* sort of machinery somewhere, or to make *very very* timing-dependent tests, which ain't great14:56
rogpeppedimitern: how can we guarantee that?14:56
rogpeppedimitern: timing isn't guaranteed, as fwereade says above14:56
rogpeppedimitern: i still don't quite understand why you need getUnitOrService14:57
dimiternrogpeppe: for cases like Life, Watch and CharmURL14:57
rogpeppedimitern: why not just call State.Entity directly in those cases?14:57
dimiternrogpeppe: there's no state.Entity()14:58
dimiternrogpeppe: there's FindEntity and what's wrong with my approach?14:58
rogpeppedimitern: oh yeah, FindEntity, sorry!14:58
rogpeppedimitern: what does getUnitOrService give you that State.FindEntity doesn't?14:58
dimiternrogpeppe: better validation of expected tag kinds?14:59
rogpeppedimitern: that's already validated by canAccess, no?14:59
dimiternrogpeppe: no15:00
dimiternrogpeppe: it only checks if you can access it15:00
dimiternrogpeppe: it's not validating tag kinds15:00
dimiternrogpeppe: a bit of extra sanity checking at the expense of 3-4 lines seems like a good idea15:01
dimiternrogpeppe: and it's only done once in getUnitorService15:01
rogpeppedimitern: how could canAccessUnitOrService return true for a tag that wasn't a service or a unit?15:01
dimiternrogpeppe: when state is flawed somehow?15:01
rogpeppedimitern: i think it's worth having clear security checking boundaries, rather than just piling extra checks on like they might help15:02
dimiternrogpeppe: you have a unit that has an invalid service attached to it?15:02
rogpeppedimitern: there are hundreds of ways we could go wrong if our code is flawed15:02
rogpeppedimitern: you're *already* assuming that canAccessUnitOrService is operating as expected15:03
dimiternrogpeppe: I didn't say our code is flawed15:03
dimiternrogpeppe: I said the state in mongo might be invalid15:03
rogpeppe[16:01:58] <dimitern> rogpeppe: when state is flawed somehow?15:03
dimiternrogpeppe: ^^15:04
dimitern(to many things called "state")15:04
rogpeppedimitern: even if the state in mongo is invalid, your checks would still not fail15:04
noodles775rogpeppe: for later/tomorrow/next week, I've also updated the interspersed flags based on our conv. yesterday: https://codereview.appspot.com/1260304715:04
rogpeppedimitern: there's no way we can return a *Unit from a service- tag15:05
rogpeppenoodles775: thanks15:05
dimiternrogpeppe: ok, I give up15:05
rogpeppedimitern: i'm only trying to get us to write less, simpler, code15:05
dimiternrogpeppe: I'm getting rid of getServiceOrUnit and replacing it with FindEntity15:05
rogpeppedimitern: cool, thanks15:05
dimiternrogpeppe: I want to land this already15:05
dimiternrogpeppe: but will keep the getUnit and getService helpers15:06
rogpeppedimitern: yeah, i thought they were worth it for the static type conversion15:06
rogpeppes/conversion/return15:07
=== tasdomas is now known as tasdomas_afk
dimiternrogpeppe: reproposing with that change15:08
dimiternrogpeppe: done15:09
=== BradCrittenden is now known as bac
dimiternrogpeppe: was that the only issue that needed fixing?15:13
dimiternmgz: thanks15:13
rvbarogpeppe: I think that review is for you :).  Nothing urgent though, it's just a clean-up. https://codereview.appspot.com/12861043/15:14
rogpeppedimitern: reviewed. thanks for bearing with me!15:14
fwereadenoodles775, just looking at https://codereview.appspot.com/12603047/patch/11001/1200415:15
fwereadenoodles775, what extra args were you expecting for debug-log15:15
fwereade?15:15
dimiternrogpeppe: thanks15:16
rogpeppervba: LGTM15:16
rogpeppefwereade: well, debug-log does say that it accepts ssh args15:18
rogpeppefwereade: which seems weird to me, but there y'go15:18
fwereaderogpeppe, seems weird to me too15:18
noodles775fwereade: I wasn't expecting any... but debug-log apparently doesn't embed CommandBase (at least, I got a compile error saying it wasn't implementing the iface due to the missing method).15:18
rogpeppefwereade: do we know what people might use that feature for?15:19
noodles775fwereade: so I needed to add the method - happy to update it to instead return CommandBase.AllowInterspersedFlags which defaults to true).15:19
fwereaderogpeppe, afraid not... wallyworld_ implemented it iirc15:19
rogpeppefwereade: does the python version allow arbitrary ssh args there, d'ya know?15:20
fwereaderogpeppe, the only case I can think of is needing to pass args down to ssh to cause that to work, but that seems surprising... an indicator of a different bug maybe?15:20
fwereaderogpeppe, looks like it doesn't15:21
fwereaderogpeppe, OTOH it *does* accept a bunch of other args that we don;t15:22
rogpeppefwereade: ha ha15:22
fwereaderogpeppe, including, it seems, "--replay" equivalent to your requested "--all"15:22
fwereaderogpeppe, although I think I still disapprove ;p15:22
rogpeppefwereade: i will definitely use --all most of the time, despite the bandwidth15:23
fwereadenoodles775, cool, I think if we just make it match CommandBase I'll be happy15:23
rogpeppefwereade: it would be nice to have a "last x minutes" flag though15:23
fwereaderogpeppe, not to mention filtering15:23
rogpeppefwereade: yeah, that too15:24
fwereadenoodles775, reviewed, LGTM with that change15:24
fwereadenoodles775, thanks15:24
noodles775Thanks fwereade, updating now.15:25
rogpeppenoodles775: reviewed15:28
rogpeppefwereade: cmd.ParseArgs can go, yay!15:29
rogpeppenoodles775: i'm very happy how that turned out15:29
noodles775rogpeppe: yeah, much nicer - thanks for the suggestions.15:29
noodles775rogpeppe: erm, you meant s/false/true when you said "I'd just return false here" didn't you? (as per fwereade, we just want the same behavior as CommandBase)15:32
rogpeppenoodles775: oh, possibly, yes, assuming we *don't* want to allow arbitrary ssh args to be passed through15:33
noodles775Right.15:33
noodles775rogpeppe: sorry, another question - when you say that ParseArgs can now be removed, are you meaning I should add SuperCommand.AllowInterspersedFlags() returning false, or is there another reason why it'll just work that I'm missing?15:43
noodles775rogpeppe: nm - I see you said that later.15:46
rogpeppenoodles775: that's the main reason i'm particularly happy with this change15:47
rogpeppenoodles775: the non-interspersing parsing of supercommand was a hack, but now it just falls out naturally.15:48
noodles775Great :-)15:48
rogpeppenoodles775: it also means that supercommands can now nest nicely if we want them to15:48
natefinchtrying to land a branch, but bzr is saying it can't get the lock on the branch.... from natefinch@bazaar.canonical.com.  That seems bad.16:00
natefinchMaybe I'm doing something wrong16:00
natefinchrogpeppe, dimitern, mgz:  any thoughts on ^16:05
rogpeppenatefinch: can you paste the message you're getting?16:06
natefinchnate@Asgard:~/code/src/launchpad.net/juju-core$ bzr push lp:~natefinch/juju-core/juju-core16:06
natefinchUnable to obtain lock  held by natefinch@bazaar.launchpad.net on taotie (process #25639), acquired 9 minutes, 56 seconds ago.16:06
natefinchSee "bzr help break-lock" for more.16:06
natefinchbzr: ERROR: Could not acquire lock "(remote lock)": bzr+ssh://bazaar.launchpad.net/~natefinch/juju-core/juju-core/16:06
_mup_Bug #25639: gs-gpl - .getdeviceparams gets called with broken types <gs-gpl (Ubuntu):Invalid> <gs-gpl (Debian):Fix Released> <https://launchpad.net/bugs/25639>16:06
rogpeppenatefinch: why are you naming your branch "juju-core"?16:06
natefinchI was attempting to follow a document Frank gave me - https://docs.google.com/document/d/1G74QKbJqPLooEBVPOeB5qgJSQ5fb7KGpmUTTjIK0NnQ/edit16:07
natefinchrogpeppe: I may have done something wrong :)16:07
rogpeppenatefinch: ah, i think those instructions are misleading16:08
rogpeppenatefinch: we usually make a unique (to the user) name for each new branch16:09
rogpeppenatefinch: (personally, i use branch names of the form 999-description, so i can remember the order i created them in)16:09
natefinchI tried that... at least, I thought so.  I had named it 002-bootstrap or something like that.  Obviously not quite the right thing16:10
rogpeppenatefinch: ah, actually that is suggested in that document: "bzr push --remember lp:~themue/juju-core/001-my-useful-change"16:10
rogpeppenatefinch: when you write "bzr push lp:~natefinch/juju-core/juju-core" you're trying to push it to a branch named "juju-core" in your lp space16:10
rogpeppenatefinch: which seems to exist already16:10
dimiternnatefinch: I saw that yesterday16:11
dimiternnatefinch: note - it's better never to interrupt lbox (^C) once started16:11
natefinchdimitern: I may have done that once or twice :/16:11
dimiternnatefinch: just do bzr break-lock bzr+ssh://bazaar.launchpad.net/... (the branch it told you)16:11
dimiternnatefinch: and then repropose, should work16:12
rogpeppenatefinch: i've also got a lock held16:12
rogpeppeUnable to obtain lock  held by rogpeppe@bazaar.launchpad.net on taotie (process #30432), acquired 1205 hours, 16 minutes ago.16:12
_mup_Bug #30432: skim is hung on with "sudo kate" <skim (Ubuntu):Invalid> <https://launchpad.net/bugs/30432>16:12
rogpeppeSee "bzr help break-lock" for more.16:12
dimiternnatefinch: and yes, it's better to pick some names for your branches16:12
rogpeppenatefinch: :-)16:12
rogpeppenatefinch: it's actually quite useful, because it's on lp:~rogpeppe/juju-core/trunk16:13
natefinchdimitern: I tried to... I think I just misread something somewhere16:13
rogpeppenatefinch: and the lock message warns me that i've forgotten to rename my branch again16:13
rogpeppenatefinch: are you using cobzr?16:14
natefinchrogpeppe: no, but only because I don't know what it is :)16:14
rogpeppenatefinch: http://labix.org/cobzr16:15
rogpeppenatefinch: some people hate it16:15
rogpeppenatefinch: i find it works pretty well for me16:15
natefinchrogpeppe: man that guy is prolific :)16:15
rogpeppenatefinch: ain't it so16:15
rogpeppenatefinch: i define bzr as cobzr (i use a shell function)16:16
natefinchrogpeppe: ahh, this was what mgz was using yesterday... I was wondering how he did that switching between branches16:16
rogpeppenatefinch: i think you can use a bzr feature too, but i don't know how to do that16:16
rogpeppenatefinch: i usually do: cobzr branch lp:juju-core/trunk16:17
rogpeppenatefinch: then: cobzr switch trunk16:17
rogpeppenatefinch: then: cobzr checkout -b 123-my-new-branch16:17
rogpeppenatefinch: then edit the branch, commit, and lbox propose16:17
natefinchrogpeppe: that looks pretty good16:17
rogpeppenatefinch: i can switch between branches with cobzr switch branch-name16:17
rogpeppenatefinch: and i can keep the same GOPATH for most development purposes16:18
rogpeppenatefinch: (i actually keep another one for testing against different go versions)16:18
natefinchrogpeppe: yeah, that was immediately my thought... having multiple branches in the same directory is a big help.  I've been renaming ones I'm not actively working on, but that's kind of a headache16:19
rogpeppenatefinch: i've got 307 branches in my juju-core tree currently :-)16:19
* rogpeppe should probably do some garbage collection some time16:20
natefinchrogpeppe: disk space is cheap :)16:20
rogpeppenatefinch: yeah, it's only 54MB16:20
mgznatefinch: no, I was using native colo16:22
mgzit's got a few quirks, but is far less buggy16:23
natefinchmgz: I saw it was an upcoming feature.. what version of bzr are you on?16:24
mgznatefinch: the one in the current distro/ppa for older versions16:25
natefinchmgz: ahh yeah, it looks like my version (2.6) has it.  Cool.16:26
mgzso, `apt-add-repository ppa:bzr/ppa` if you're on precise or something16:26
mgzright, 2.6 is what you need16:26
=== natefinch is now known as natefinch-lunch
natefinch-lunchback in a bit... thanks for the help16:29
=== natefinch-lunch is now known as natefinch
natefinchmgz, rogpeppe, dimitern: can one of you (or anyone else) help me detangle my bzr mess and get my branch landed?17:10
rogpeppenatefinch: sure17:10
natefinchrogpeppe: thanks17:10
ahasenackhm, since when does juju bootstrap perform a tool sync from amazon?17:10
natefinchrogpeppe: so, I think this is probably one problem:17:10
natefinchRepository tree (format: 2a)17:10
natefinchLocation:17:10
natefinch  shared repository: /home/nate/code17:10
natefinch  repository branch: .17:10
natefinchRelated branches:17:10
ahasenackhttp://pastebin.ubuntu.com/5981915/17:10
natefinch    push branch: bzr+ssh://bazaar.launchpad.net/~natefinch/juju-core/002-notbootstrapped/17:10
natefinch  parent branch: /home/nate/code/src/launchpad.net/juju-core-trunk17:10
natefinch  submit branch: bzr+ssh://bazaar.launchpad.net/+branch/juju-core/17:10
rogpeppeahasenack: since quite recently, but i don't think that should happen when using --upload-tools17:11
mgznatefinch: what's the mess?17:11
ahasenackrogpeppe: yeah, and it's syncing the wrong tools even (1.12)17:12
natefinchmgz: see the paste above... that looks wrong, but I don't really know what it's supposed to look like, so maybe that's ok17:12
rogpeppeahasenack: looks odd to me17:13
mgznatefinch: apart from submit branch (which doesn't really matter) it's fine?17:13
rogpeppeahasenack: it should find the tools it's uploaded17:14
natefinchmgz: oh, ok.  I thought the submit branch was what was tripping me up.17:14
ahasenackrogpeppe: fwiw I'm on raring, and requested a saucy bootstrap node, but it did say it was uploading a saucy tarball17:14
ahasenackI'm filing a bug17:14
rogpeppeahasenack: +117:14
natefinchmgz: so, how do I land this thing? I merged from trunk, so I should be up to date17:15
mgznatefinch: see my "Landing bot active again" post to juju-dev for an easy way17:17
mgzin the archives, july 29th17:17
natefinchmgz: where's the archive?  I wasn't on the mailing list at that point17:18
mgzlists.ubuntu.com17:18
natefinchmgz: nice, thanks17:19
fwereadesidnei, reviewed17:27
rogpeppedimitern, fwereade, natefinch: fairly trivial change to utils/set: https://codereview.appspot.com/1287204317:34
sidneifwereade: uhm, not sure i get the comment about instance types without instance-storage? what are those?17:37
=== tasdomas_afk is now known as tasdomas
=== tasdomas is now known as tasdomas_afk
rogpepperight, environs/ec2 tests pass. time to stop for the day.17:38
rogpeppeg'night all17:38
natefinchrogpeppe: LGTM'd17:40
=== BradCrittenden is now known as bac

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