/srv/irclogs.ubuntu.com/2013/04/02/#juju-gui.txt

hatchcan do!00:00
gary_posterhatch, after that, expose and unexpose would be easy things to do, I bet.00:00
hatchalright assigned00:00
gary_postercool, thank you!  honestly, the only thing particularly tricky there that I see is the annotations, and that is just because we will need to hook up annotation deltas00:01
gary_posterbut that should also follow existing patterns00:01
hatchsounds good00:03
frankbanmorning rogpeppe: does juju-core trunk compile for you? I see cmd/builddb/main.go:9: imported and not used: "launchpad.net/juju-core/state"09:15
rogpeppefrankban: i see that too. it should be fixed.09:15
rogpeppefrankban: nothing depends in builddb though, i don't think, so it's not critical09:15
frankbanrogpeppe: doesn't it prevent juju-core to be "go install"ed?09:17
rogpeppefrankban: no. just builddb09:17
frankbanrogpeppe: ok09:18
frankbanrogpeppe: unrelated, what's your plan on including constraints and settings in the allwatcher? is there anything I can do to help implementing it?09:20
rogpeppefrankban: i guess the only thing to do is add new Info types and add their collections to the set of collections watched09:21
rogpeppefrankban: it's a bit weird that constraints are in their own collection09:21
rogpeppefwereade__: why are constraints in their own collection?09:21
frankbanrogpeppe: because constraints are only related to units?09:21
rogpeppefrankban: no, constraints are related to units, services and machines09:24
rogpeppefrankban: the difficulty is that neither constraintsDoc nor settingsDoc hold the entity name in the doc09:25
rogpeppefrankban: well, actually there's no settingsDoc per sre09:26
rogpeppes/sre/se/09:26
frankbanrogpeppe: yes, the tag09:26
frankbanrogpeppe: AFAICT constraints include the global key (pk) and settings something else (e.g. serviceSettingsKey)09:29
rogpeppefrankban: yes - i think the service settings key is to allow charm upgrades09:30
rogpeppefrankban: but i don't know much of the detail09:30
frankbanrogpeppe: so we need a way to include the entity tag in both docs09:31
rogpeppefrankban: yeah, that's probably the easiest way forward. but might not go down well. i'm having another look at adding a level of indirection.09:32
frankbanrogpeppe: cool, thanks09:32
fwereade__rogpeppe, frankban, hey, sorry09:33
frankbanmorning fwereade__ 09:33
fwereade__rogpeppe, frankban: basically they're in their own collection because they're conceptually distinct from the various entities and they're not fundamental to the operation of the entities in question09:34
fwereade__rogpeppe, frankban: consider also settings09:34
rogpeppefwereade__: aren't they attributes of the entities in question?09:35
fwereade__rogpeppe, I'm not sure there's anything strongly indicating that they must be09:35
rogpeppefwereade__: it seems a bit weird that you can do Service.Watch but you don't see the service constraints change09:35
rogpeppefwereade__: well, we could have every attribute in its own collection09:36
rogpeppefwereade__: i'm not sure that would be good though09:36
fwereade__rogpeppe, well, I'm not proposing that...09:36
rogpeppefwereade__: i'm not sure what you think is good about having attributes of an object spread across separate collections.09:37
fwereade__rogpeppe, well, it's not exactly unheard of... consider annotations, settings, scope membership...09:40
fwereade__rogpeppe, it is not always neatest or cleanest to stick everything related to X into the document for X09:41
rogpeppefwereade__: annotations and settings have a good reason (they can be unbounded in size)09:41
rogpeppefwereade__: i don't know about scope membership09:41
fwereade__rogpeppe, that's mainly about being able to watch sanely09:41
rogpeppefwereade__: watch scope membership?09:42
fwereade__rogpeppe, the real underlying worry is that the more overlapping txns we have on given objects the more contention we'll have09:42
rogpeppefwereade__: i worry that we're prematurely optimising here.09:42
fwereade__rogpeppe, yes; and as a corollary *not* get events on the unit every time something irrelevant happens09:42
rogpeppefwereade__: we're making all kinds of guesses about efficiency09:43
rogpeppefwereade__: and we don't know anything about the likely distribution of db ops in a large juju environment09:43
fwereade__rogpeppe, I'm writing the code in the most natural way I see, in accordance with things we've done in the past, and in general a feeling that just slapping everything into one giant object is a poor way to go by default09:43
rogpeppefwereade__: putting things into separate collections has costs too though.09:45
fwereade__rogpeppe, sure; what are the ones that particularly concern you?09:45
rogpeppefwereade__: n fetch ops vs 1.09:46
rogpeppefwereade__: seeing inconsistent data09:46
fwereade__rogpeppe, well, the point is to chunk them such that n=1 most of the time, and reap the benefits of sending only the data you need09:47
fwereade__rogpeppe, and similarly, if related data goes together, you don't see inconsistency there09:48
rogpeppefwereade__: perhaps there's is no problem in fact. the issue does concern me though.09:50
fwereade__rogpeppe, I have tried to plot a sensible course between the two obviously bad extremes -- single-giant-object vs collection-per-attr09:52
fwereade__rogpeppe, I doubt the course is perfect ofc09:54
rogpeppefwereade__: it's probably fine. it's just that every time i turn around there seems to be another collection.09:55
fwereade__rogpeppe, but as a heuristic I feel that, if there's no overlap between the users of field A and field B, we should at least consider putting them in separate docs09:56
fwereade__rogpeppe, yeah, I know the feeling, but they're actually pretty useful building blocks09:56
rogpeppefwereade__: i feel that there's something to be said for the conceptual integrity of having attributes that relate to an object in the same place.09:57
fwereade__rogpeppe, IMO wedging unrelated data into a single object is one of the leading causes of decay to conceptual integrity :)09:59
fwereade__rogpeppe, but I kinda feel we're into aphorism territory here09:59
rogpeppefwereade__: yeah09:59
fwereade__rogpeppe, I think I do understand your concerns; I still think I've made a reasonable choice, but I'm also interested to hear any pain points you encounter because of it, so that I can make better choices in future10:02
fwereade__rogpeppe, thanks :)10:02
rogpeppefwereade__: i will do. np.10:02
frankbanrogpeppe: so, how do we proceed? are you thinking about decoupling the EntityInfo (sent as part of the delta) and the entity docs?10:21
rogpeppefrankban: yes, that's what i'm going now.10:22
rogpeppes/going/doing/10:22
rogpeppefrankban: i've made the most important change in the code (the decoupling) and am just figuring out the test changes now.10:22
frankbanrogpeppe: cool, thank you10:23
frankbanrogpeppe: in an environment bootstrapped using --upload-tools (quantal) i deployed cs:precise/mysql. the status is stick to pending, in the machine log I see http://pastebin.ubuntu.com/5670077/11:00
rogpeppefrankban: hmm, that's the issue that dave cheney saw11:00
rogpeppefrankban: i couldn't reproduce it11:01
frankbanrogpeppe: it seems it only happen when the series of the bootstrap node and the service does not match. I tried to deploy a quantal charm (cs:~charmers/quantal/ceph-3) and I "correctly" have an install error11:02
frankbanrogpeppe: however, I remember I was able to deploy a precise charm using --upload-tools (quantal) last week11:04
rogpeppegary_poster: it seems that i can't put off adding that layer of indirection to the allWatcher12:21
rogpeppegary_poster: the code is easy, but changing the tests is taking a little while12:21
gary_posterrogpeppe, is this necessary for the config and constraints, and future state stuff?12:22
rogpeppegary_poster: config, constraints, status and probably more stuff too12:22
gary_postergotcha12:23
gary_posterrogpeppe, will this indirection branch add the config and constraints or is it a step along the way12:23
rogpeppegary_poster: it's a step along the way12:23
gary_posterok (and darn :-) )12:23
rogpeppegary_poster: but adding the other stuff should be "easy" (modulo type renaming :-])12:24
gary_posterheh.  rogpeppe, I suspect we ought to have a call today to see where we are, what more we need, and what the timeline is12:24
rogpeppegary_poster: agreed. any time. apart from i've got a call in 35 mins.12:25
gary_postercool rogpeppe. I'm hanging out in http://tinyurl.com/guichat and will be ready to talk whenever you are12:26
frankbangary_poster: do you have time for a quick call?12:41
gary_posterfrankban, sure.  come on by guichat12:42
benjiso much email12:43
rick_h_benji: bwuhahaha, motivation to never leave12:50
benjiheh12:50
benjigary_poster: I'm trying to figure out the status of my "Add relationship attributes to watcher" card.  Do you know anything about it?  Did anyone work on it?12:51
gary_posterbenji sent you email.  short answer: no progress, you take it12:52
rick_h_benji: thought gary_poster sent an email last night. Bottom of your pile :)12:52
rick_h_doh, too slow12:52
gary_poster:-)12:52
benjiI read them all so I must have not understood.  12:52
benjiI'll go back and read it again.12:52
gary_posterbenji, look for reply to your status email.  12:54
gary_poster"""I didn't touch this, but I added a card for it (I appropriated the one12:54
gary_posteryou and Francesco were using for my branch).12:54
gary_posterI also added a card for "add config and constraints to event watcher" to12:54
gary_poster"Ready to code""""12:54
rick_h_benji: "Re: More info in deltas" is the subject12:54
gary_posterbiab12:54
rick_h_<3 notmuch12:54
gary_poster:-)12:56
gary_posterfrankban, ok ready now :-)12:57
gary_posterbenji, all cool?12:57
benjigary_poster: yep!  I missed the one line I was looking for.  Reading for comprehension...12:58
gary_poster:-)12:58
gary_posterfrankban, in guichat again12:59
rogpeppegary_poster: i've just realised my regular call has shifted by an hour. let us know when you're done with frankban, and i'll G+ you.13:08
gary_posterok thanks rogpeppe13:08
teknicoright! both sides of the pond in sync again, so daily call in 1:15-ish, right?13:13
rick_h_teknico: rgr13:13
gary_posterrogpeppe, could you join frankban and me in https://plus.google.com/hangouts/_/7fb7c30f3a232db57dd8549738fb98e723d90d4a ?13:25
hatchgoooooood mornin13:56
hatchgary_poster: so are you the only one that gets the CI emails now?13:56
bachatch: i got a jenkins failure email13:58
hatchhmm interesting - I only got the one from gary13:58
hatchmaybe it got lost in the internets13:58
hatchwill see if I get the next one13:58
gary_posterhatch, it was a very big email.  maybe filtered out?13:58
gary_posterLooks like canonistack is hosed yet again 13:59
gary_posterGoing to kill this one too13:59
* hatch grumbles13:59
gary_posterno resources is my guess13:59
gary_posterwe are only getting a single machine13:59
hatchso EC2 would cost us $X/yr could we requisition that $ and buy a box just for us in Canonistack? :D14:00
gary_poster:-)14:00
hatchI'm only a little bit joking :)14:00
gary_posteryeah, I regard building these stats as grist for the mill of getting this fixed one way or another14:00
gary_posterI'll be able to say "we have had X runs of the CI in one week. Y% failed because of canonistack.  We'd like to fix this or be able to go someplace else."14:01
bachatch: if it costs $X to buy a box it'll cost about $100X in time trying to get it purchased and installed.14:01
gary_postertoo true :-(14:01
hatchbac: isnt' that what juju is for....:P14:01
gary_posterno, that's what the cloud is for "=/14:02
gary_poster:-/14:02
hatchhaha14:02
gary_posteroff by one smiley14:02
bacsince benji has such good internet, why don't we just host a box at his house?14:02
* benji considers adding UPS and a raised floor.14:03
gary_posterhey, that's a great idea! We can all get root on his network!14:03
rick_h_hah, everyone loves those HP microservers14:03
rick_h_a shelf full of those should be the awesome!14:03
rick_h_http://www.jorgecastro.org/2012/04/10/maasing-and-jujuing-with-hp-proliant-micro-servers/14:03
rick_h_looks like we need to ping robbie for access to robbiestack14:04
rick_h_luca__: howdy, I just sent an email to alejandra about a possible meet up today. I meant to copy you and sinzui but suffered email fail of the brain. 14:05
baclate but good: http://youtu.be/9shZslfbaS014:07
luca__Hi rick_h_ no problem, I'm available for a catch up but will let Ale reply. We'll be on the standup so we can sort it out then.14:09
benjithat's pretty good14:13
rick_h_bac: I like the "how many blocks are supposed to be in this iteration?" lol14:13
gary_posterhey look, the tests are actually runnning now!  how novel14:14
gary_posteron canonistack I mean14:14
gary_posterugh!14:23
gary_posterand they failed because of bug 1161937.  ugh.14:24
_mup_Bug #1161937: Unit tests fail in CI initially, intermittently <juju-gui:Triaged> < https://launchpad.net/bugs/1161937 >14:24
hatchbac: does trunk have our fix from yesterday?14:24
gary_posterno14:24
bacno14:24
hatchoh ok - I was thinking that 'might' fix it?14:24
bachatch: i can pull it into a separate branch and land it14:25
hatchI am almost certain those failures are caused by something along the same lines as we solved yesterday14:25
gary_posterjujugui, call in 214:28
gary_posterfrankban, starting without you14:31
frankbangary_poster: koining14:32
frankbanjoining even14:32
benjibac: that was the best update ever14:44
hatchlol14:45
luca__rick_h_: https://plus.google.com/hangouts/_/8dff1ab85e077e6f0c07a33419a091a66c4536d5?authuser=1&hl=en14:47
gary_posterbcsaller, what's the earliest you would like a regular call time?15:08
bcsallergary_poster: even 8am would be an improvement, a 1/2 hr later15:11
gary_posterthat's not the question I asked though ;-) 15:12
gary_posterbcsaller, ^^15:13
bcsallergary_poster: 9am would be my preference but I don't want it to be too late in Europe 15:13
gary_posterack.15:13
gary_posterI could propose M-Th @ 9 and Fri @ 8 bcsaller?15:15
bcsallergary_poster: deal :)15:15
gary_postercool :-)15:15
gary_posterhey jujugui.  bcsaller is in a new timezone that means the current daily call time is very early for him.  Does anyone have objections to the following: daily call M-Th, UTC 1600.  weekly call Fri, UTC 1500.15:17
Makyogary_poster, Works for me.15:18
gary_posterthx15:18
benjithat's fine with me15:18
frankbangary_poster: ok15:19
gary_postercool15:19
hatchiunnoooo15:19
hatch:P15:19
teknicogary_poster, fine with me too15:19
gary_posterthx teknico.  hatch, issue?  let me know if there's a prob15:19
hatchno no issue :) that's 10am and 9am here so I should be well into working by then15:20
gary_posterok cool15:20
hatchbcsaller: just so we know when we can bug you - what time UTC do you start now?15:21
hatchrick_h_: is there a way to copy the text from one panel in a split panel window in iterm?15:22
hatcher tmux in iterm15:22
gary_posterbac is not here15:22
gary_posterhoping it is ok with him15:22
hatchgary_poster: he is on the same timezone as me so I don't think it would be an issue15:23
gary_postercool15:23
hatchgary_poster: to write the remove_units() fakebackend I executed the command on improv and then took note of the req/res and will duplicate that15:24
hatch^^ correct approach?15:24
gary_posterhatch, good start, yes.  For response, you'll want to look at callbacks as well.  Sometimes improv is not as complete15:24
hatchright gotcha15:25
hatchrick_h_: I found the answer http://unix.stackexchange.com/questions/58763/copy-text-from-one-tmux-pane-to-another-using-vim15:30
bcsallerhatch: I think I'm an hour behind you, its 8:30am here now . I usually try to start around now.15:32
hatchahh ok cool15:32
hatchGMT-7 or UTC 15:3015:32
hatch;)15:32
hatchwe should schedule everything in UTC time :)15:33
hatchgary_poster: how did you know to add in all of these if checks in addUnit? Did you also look at improv?15:36
* gary_poster looks15:36
hatchsome of it is similar to service.js but not all of it15:37
hatchok nope add_unit.py doesn't have those checks in it either15:42
hatchlooks like you were just being thorough15:42
* hatch just doesn't /get/ yield in python15:52
benjihatch: to get yield you first have to get generators.  A generator is like a function that instead of returning one thing and stopping can return something and then later be asked for another (and another, etc.)15:56
hatchso you pass it an array and every time it's called it moves the pointer forward?15:56
benji"yield" is like "return" except that it means that you can be asked again for another value and you will yield again15:57
benjinope, "yield" yields whatever you pass it (just like return)15:57
hatchok then that's the issue - i don't understand what the difference is...where does this 'next' value come from?15:58
hatchif your passing new data in... then how is it any different than a function with return15:58
benjithe value passed to yield is what next receives15:59
benjiyou don't normally pass in new values to each "iteration" of the generator16:00
benjinormally you pass in values to get it started and then it generates a series of values; every time you call next the generator is run until it yields a value and that is the return value of the next call16:00
hatchohh ok ok now I get it16:01
hatchthanks for that :)16:01
gary_posterhatch, sorry, I looked, and then was pulled down two rabbit holes.  I climbed back out of one of them, but it looks like you are all taken care of.16:02
hatch:) no problem16:03
benjihatch: here is a little generator you could play with: http://paste.ubuntu.com/5670872/16:04
hatchmust....have....curly......braces.....16:05
hatch:P16:05
benji:)16:05
benjiIt will be no surprise that I feel exactly the opposite. :)16:06
hatchhaha nope - My mind just looks for the closing brackets to compile in my head16:08
hatchpython throws a brain syntax error16:08
hatch:P16:08
=== matsubara is now known as matsubara-lunch
teknicohatch, then there's "yield from": ;-) http://docs.python.org/3/whatsnew/3.3.html#pep-380-syntax-for-delegating-to-a-subgenerator16:24
rick_h_luca__: if you get a sec can you pull up the 'weekly' update that last went out and shoot a copy to me/sinzui?16:25
benjirogpeppe: I am having some test failures that I could use some help on.  I figured several out but one is still getting me: http://paste.ubuntu.com/5670930/16:26
rogpeppebenji: looking16:26
benjithe problem seems to be that setUpScenario sets up less than what is found, which doesn't make sense to me16:27
rogpeppebenji: the problem is *probably* that some test op hasn't undone a remove-relation change properly16:27
rogpeppebenji: i thought some branches relevant to that had already been submitted (though i can't quite remember who was doing them. teknico? frankban?)16:28
benjihmm, that is a possibility16:28
benjithis branch hasn't merged trunk since Friday, so maybe a merge will magically fix this (after breaking the world with merge conflicts, but I can't have everything)16:29
benjiI'll give that a try and if it doesn't help I'll go on a search and destroy mission for an improperly un-done test.16:29
rogpeppebenji: y'never know16:29
frankbanrogpeppe: I think teknico did that kind of clean up16:30
rogpeppebenji: a quick check:16:30
benjihmm, but the test generates an entirely new scenario, how could another test interfere?16:30
rogpeppebenji: what does opClientDestroyRelation look like?16:30
benjifunc opClientDestroyRelation(c *C, st *api.State, mst *state.State) (func(), error) {16:30
benjierr := st.Client().DestroyRelation("nosuch1", "nosuch2")16:30
benjiif api.ErrCode(err) == api.CodeNotFound {16:30
benjierr = nil16:31
benji}16:31
rogpeppebenji: ah, i don't know which function your test failure relates to16:31
benjireturn func() {}, err16:31
benji}16:31
rogpeppebenji: perhaps you could push your branch and i'll take a look16:31
* benji notes another point against tabs in source.16:31
rogpeppebenji: that looks fine16:31
luca__rick_h_: the last email that me or Jovan has from Nick is from the 1st of March. Do you have that one?16:31
rogpeppebenji: it's ok, gofmt will put 'em back :-)16:31
teknicoit looks fine to me too16:32
benjire. tabs: oh, I know16:32
teknicoI added reset() (cleanup) functions to a number of tests16:32
* rogpeppe likes tabs. only one key to indent :-)16:32
benjiteknico: cool, I'm just a little surprised that this function is dependent on global state16:33
rogpeppebenji: which function is failing?16:33
benjifunc (s *allWatcherStateSuite) TestStateBackingGetAll(c *C) {16:33
* teknico likes them too. Used them for months when starting with python eons ago, before being forced to use spaces :-P16:33
rogpeppebenji: ah, hmm. i'd unwittingly assumed it was api tests failing16:34
benjiI don't think most editors have a problem inserting spaces when you hit the tab key.  Tab key != tab character.16:35
rogpeppebenji: ah! i see the problem.16:35
benjiyay!16:35
gary_posterhatch, sorry, I looked, and then was pulled down two rabbit holes.  I climbed back out of one of them, but it looks like you are all taken care of.16:35
rick_h_luca__: heh ok. So by 'weekly' we've actually not missed a lot? luca__ but no, I don't have an email from nick on march 1.16:35
gary_posteroh bah16:35
gary_postersorry hatch16:35
rogpeppebenji: you've just added new attributes to RelationInfo, right?16:36
benjiyep16:36
gary_posterthat was supposed to be a different message and my irc client overrode it when I pressed the up key inadvertently16:36
* hatch confused16:36
rogpeppebenji: so you'll need to change allWatcherStateSuite to add the relevant params to the expected added relation16:36
hatchoh heh got it16:36
luca__rick_h_: hmmm do you have one from Nick sent on the 19th?16:37
rogpeppeallWatcherStateSuite.setUpScenario of course16:37
gary_posterfrankban, when I get an error in the charm like line 12 of http://pastebin.ubuntu.com/5670949/ (ignoring line 13's timeout for the moment) what can I do to diagnose?  The only think I know to do is to try and dupe manually, or rerun with debug-log running.16:37
rogpeppebenji: ^16:37
benjiI'm confused as to why that should need to be done, but now is probably not the time to relieve that confusion.  Thanks!16:37
rogpeppebenji: setUpScenario adds stuff to the environment and also returns the stuff we *expect* getAll to fetch.16:38
benjiah!16:38
benjibut, isn't the thing it returns generated from the things it creates?  or are those two entirely distinct?16:39
rogpeppebenji: it's hopefully slightly easier to maintain than a large section of code that adds stuff and a large slice with all the stuff that pertains to the code.16:39
arosalesgary_poster: are you targeting have the gui on the tablet running by ODS?16:40
rogpeppebenji: they're almost entirely distinct16:40
rogpeppebenji: note that although each change has an equivalent add() call, the add call doesn't interrogate the state.16:41
frankbangary_poster: it usually prints a traceback. when it doesn't, it probably means jitsu timed out. In general, debug-log helps a lot, and if a single test fails it can be helpful to manually try to do what the failing test does, i.e. deploy the charm with staging=true.16:41
gary_posterarosales, that's a nice to have16:42
gary_posterfrankban, ack, thank you16:42
gary_posterarosales, want a quick check-in since I'm out the next three days?16:42
arosalesgary_poster: sure, I have some time this afternoon.  I will look for an open spot16:43
gary_postercool16:43
=== matsubara-lunch is now known as matsubara
benjirogpeppe: I have a problem, adding those constants to the expected values will create an import loop16:57
benjiI don't really expect you to do anything about it, just complaining. 16:58
rogpeppebenji: which constants?16:58
benjithe endpoint data16:58
rogpeppebenji: that's a bit surprising17:00
rogpeppebenji: could you paste me the line(s) that cause the loop?17:00
rogpeppebenji: AFAICS you don't need anything that's not from state/api/params or charm, but i'm probably missing something crucial17:01
benjirogpeppe: I don't have a loop yet, I just expect one.  Let me go down this road and see if the loop materializes.17:02
frankbangary_poster: EOD, and I have a prototype/draft of the model handlers/converters. Here is the current diff: http://pastebin.ubuntu.com/5671075/ . If you have time, I'd appreciate some feedback (email) before a continue following this path. Otherwise, no problem. In both cases, have a great week! :-)17:26
=== deryck is now known as deryck[lunch]
gary_posterthanks frankban, I will look later.  Hope you have a good week too :-)17:26
hatchgary_poster: do you have a minute for an impromptu review?17:36
gary_posterhatch, very fast :-)17:36
hatchhttp://bazaar.launchpad.net/~hatch/juju-gui/remove-units/revision/48317:36
gary_posterlooking17:37
hatchthanks :)17:37
rogpeppegary_poster: here's the indirection branch: https://codereview.appspot.com/8269043/17:37
rogpeppegary_poster: which brings me to eod17:38
rick_h_hatch: jcsackett Makyo sinzui or anyone else that's feeling in a good mood up for a smallish review please? https://codereview.appspot.com/8268043/17:38
hatchrick_h_: I'll take one17:38
gary_posteryay rogpeppe thanks!  have a good evening17:38
rogpeppegary_poster: hope you have a good time off!17:38
gary_posterthank you :-)17:38
rick_h_thanks hatch 17:38
gary_posterhatch, fakebackend.js, line 430, why some?  would expect each17:39
gary_posteruse in line 431 makes sense17:39
gary_posteryou never return true in outer loop so some is effectively each17:40
hatchgary_poster: oh woops, I used to have those two loops inverted so that should be changed to an each17:40
hatchthanks17:40
hatchoh wait17:40
gary_postercool hatch.  You may want to aggregate errors in line 43817:40
hatchno that was there on purpose - so that it would stop if it came to an error17:41
hatchthe improv just logs errors to the console then returns false17:41
gary_posterhatch, then it stops processing?17:42
gary_posterreturns false...17:42
hatchwell it 'raises' so I am assuming?17:42
gary_posterwerid17:42
gary_posterjust like my spelling/typing17:42
hatch:D17:42
hatchmaybe you could take a look at remove_unit.py just to make sure17:42
gary_posterk17:42
hatchit throws exceptions and raises which I'm guessing exits the loop?17:43
gary_posterhatch17:44
gary_poster        except ServiceUnitStateNotFound:17:44
gary_poster            context.log.warning("Unit %r does not exist", unit_name)17:44
gary_poster            continue17:44
gary_posterso that means it ignores missing units17:44
hatchahh ok I can aggregate the errors then17:45
gary_posterand then they aren't really errors, oddly enough17:45
gary_posterjust warnings17:45
gary_postermaybe put as separate attr?17:45
hatchwell err is only checked for truthyness so I will just turn it into an array and push them into there17:46
hatchas right now it simply returns boolean and logs to the console17:46
gary_posterhatch but my point is that this is not an error condition17:46
hatchso this is actually adding more information than we had before17:46
hatchohh I see17:47
hatchI'm not sure what would be an error then17:47
benjiyay, tests pass; my import cycle fears were unfounded17:47
gary_posterhatch, only error is if unit's service is subordinate: we can't do that17:48
benjitime to see if my merge fears can come true instead17:48
hatchyeah I didn't even know what that was lol17:48
gary_posterhatch, I think service has subordinate flag17:49
gary_posterpuppet is subordinate hatch17:49
gary_posterbecause it runs on the same machine as a, uh, non-subordinate charm, and supports17:49
gary_posteryou could have a logging charm too17:49
gary_posterit is something that is supposed to be able to support ither charms17:49
hatchoh ok - so is that something I should be taking into account?17:49
gary_posterother17:49
gary_posternagios is another example17:50
gary_posterhatch, would be easy to: get the service and loog at the subordinate flag17:50
gary_posterlook17:50
hatchalright I'll be sure to add that17:50
gary_posterhatch your logic in performOp_remove_units looks a bit weird in lines 385-38617:51
gary_posterif (!res) then res.error would exist17:51
hatchit's because res is boolean17:51
gary_posterhatch, but an error object will also be truthy17:52
gary_posterI'd document the return values of removeUnits also in the pydoc thing17:52
hatchyeah so if (res.error) { data.err = res.error; } data.result = res;17:52
gary_posterhatch that would work if you changed the output of removeUnits, yes17:53
gary_posterhatch, think about what a reasonable reply from removeUnits would be in the abstract17:53
hatchyeah I think I'll try and mirror the other methods17:53
hatchto make it consistent17:53
gary_posterfor instance, I'd be tempted to return a hash of {removed: [], notFound: []} *or* {error: ...}17:54
gary_posterbut that's off the cuff17:54
hatchyeah I could do that if that's allowed17:55
hatchyou said yesterday not to return any extra data than what's already there17:55
hatchso I was trying to stick to that :D17:55
gary_posterhatch, remember, there's a difference between fakebackend and pyjujuapi17:56
hatchoh right, fakebackend can return whatever17:56
hatchpyjujuapi only return the same stuff17:56
gary_posterfakebackend is just supposed to be helpful JS thing with helpful JS API including helpful JS responses17:56
gary_posterright17:56
hatchok so check for subordinate and aggregate warnings17:57
hatchfix conditional17:57
gary_posterI assume you will use your new generateServices helper to refactor the add unit test from yesterday too17:58
gary_postergood idea17:58
hatchyep I will for sure17:59
gary_postergood work hatch, on the right track17:59
hatchthanks18:00
* gary_poster returns to reviews18:00
hatchgetting the hang of this system heh18:00
benjigary_poster: I am going to take the rest of lunch now (I was trying to get my branch done before the juju-core guys disappeared, almost made it) but I could use your advice about what to do afterward18:13
gary_postercool benji.18:13
gary_posterbenji, would be great if you could review the py env and go env commands and note any missing pieces.  We only have set-config listed on board now18:15
gary_posterwith get-config and get_endpoints in progress18:15
benjigary_poster: ok, I'll take a look18:15
gary_posterif there are any others we should make cards, and then tackle18:15
gary_posterthank you18:15
hatchrick_h_: review done18:16
rick_h_hatch: thanks!18:19
rick_h_hatch: replies sent. Thanks for the catches18:29
rick_h_and suggestion on lazy loading the attr18:29
hatchrick_h_: got it thanks18:30
hatchso...18:30
rick_h_hatch: so api is giving me: http://staging.jujucharms.com/api/0/charm/precise/cassandra-118:31
rick_h_hatch: see the options18:31
rick_h_I need to loop/output those for display through handlebars which doesn't look looping over objects, but wants arrays18:31
hatchyep I understand that18:32
rick_h_k18:32
hatchI don't think that config should be there at all18:32
hatchI'd probably move that getter into a fn18:32
hatchwhich would pull the data from the object and format it18:32
hatchas I don't think that an attribute should be used as only a getter for another attribute18:32
rick_h_hatch: well, I want this to be backwards compatible with the old charm model and I'll be passing this to the environment/config editing code from you guys18:33
rick_h_hatch: so I want to keep split things I just need for me vs changing the model api as a whole. 18:33
rick_h_hatch: though I could just move this to the view, but then I can't render the templates as model.getAttrs() :)18:33
rick_h_hatch: so maybe if you don't want the double in the model I should look at moving the logic to the view instead18:33
hatchif you set the model on the view to be this model then you could18:34
rick_h_hatch: not following that last statement18:35
hatchwell if you're in your view and you set the views model to be this model then you would go template(this.get('model').getAttrs());18:35
hatchassuming template is a hbs render function18:36
rick_h_hatch: right, so it's called 'charm' and I do that currently. template(this.get('charm').getAttrs())18:36
rick_h_but you're saying you don't like both options and config being in the model correct?18:36
hatchno no, what I mean is that I don't like an attribute being created just to format another attribute18:37
hatchto me you should be creating a parse getter method18:37
hatchgetFormattedObject: function() {}18:37
rick_h_hatch: right, but I want to keep the original object based attribute. I don't want to replace it18:37
=== deryck[lunch] is now known as deryck
hatchthat's fine - it would be accessible via this.get('object')18:38
rick_h_hatch: but you're right, what might be better is to add it as a method on the charm. charm.getOptionsAsArray18:38
hatchand getFormattedObject would return the formatted object18:38
hatchyeah - that's what I mean18:38
hatchI ran into a similar situation in a previous project18:38
rick_h_ah ok gotcha. I thought you wanted to change the parse method on the model for load/etc which isn't what I wanted18:39
hatchoh no no definitely not :)18:39
hatchso do you agree with the approach I'm suggesting?18:39
rick_h_hatch: I *think* so. I'll update here in a sec18:40
hatchalright :)18:40
rick_h_can you call methods from handlebars? I didn't think so. 18:40
rick_h_so I can't do {{#each this.getOptionsAsArray()}} right?18:40
rick_h_yea, nvm18:41
hatchonly via a helper18:42
* hatch lunching &18:57
rick_h_hatch: updated when you get time https://codereview.appspot.com/8268043/ 19:03
rick_h_jcsackett: have time to look as well please? ^^19:03
jcsackettrick_h_: looking.19:04
MakyoTurns out, these are not valid x/y coordinates: http://pastebin.ubuntu.com/5671410/19:08
hatchMakyo: rofl19:12
hatchit is actually on the cardashian plain lol19:13
gary_posterMakyo, lol19:13
MakyoJust a little side effect of storing annotations as map[string]string.19:14
hatchrick_h_: LGTM'd19:16
rick_h_hatch: thanks19:16
gary_posterbac, I don't have time for a review, but your endpointsbranch looks darn impressive on the face of it19:17
bacgary_poster: thx.  hopefully i'll get it landed today.19:18
baclots of red in the branch.  :)19:18
benjiunhelpful test failur of the day goes to:19:18
benji... obtained params.Delta = params.Delta{Removed:false, Entity:(*params.RelationInfo)(0xf8400e7420)}19:18
benji... expected params.Delta = params.Delta{Removed:false, Entity:(*params.RelationInfo)(0xf8400c8d20)}19:18
hatchbac: reviewing19:19
bacbenji: ah, those objects don't match.  you should make them be the same.19:19
benjiheh19:19
bachatch: not just yet.  i don't think the final version is on Rietveld yet19:20
baci did a -wip first19:20
hatchoh alright19:20
hatchlemme know19:20
baclbox propose is crazy slow19:20
hatchI'm getting quotes to reroof/shingle my garage roof today19:21
* hatch don't feel so good no mo19:21
bacmy roof is 18 years old.  /me keeps his fingers crossed19:21
hatch~$3500 to sheet and shingle a detached garage...19:21
hatchthat's like $2000 labour haha19:21
bachatch: now it is up: https://codereview.appspot.com/827504319:22
hatchcool19:22
hatchbac: does this need to be QA'd?19:27
bachatch: er?19:27
hatchlike a manual QA19:27
hatchmake sure everything still works as expected19:27
bachatch: are you asking if i've run it locally?  yes.19:27
bachatch: more poking at it would be appreciated19:28
hatchbac: why are you binding to null in endpoints.js ln 167? I remember you saying the reasoning but now I can't remember19:33
hatch:)19:33
bachatch: as before, b/c i don't care about the context but just need to curry in other params19:35
hatchahh yes19:35
bachatch: i see i left some commented code a few lines above.  please mark those so i don't forget to remove them19:35
bacnm, i just killed them19:36
hatchyeah I already marked em19:37
hatchso...19:38
hatchI'm a little concerned that these utility methods are just hanging off of juju.models19:38
hatchimho they should be in a closure in juju.models.utils or something19:38
hatchthoughts?19:38
bacno thoughts, open to suggestions.  they do look a little bare.19:39
hatchalright well if you're not opposed to it then I'll make a note and come back to it19:39
hatchlol if(true) wth lint19:40
hatchbac: review completed - I'd like to get some others input on moving the code into it's own closure19:57
bachatch: thanks.19:58
hatchthat's the only reason why it wasn't LGTM'd just fyi19:58
bachatch: the linter was complaining about the body of a for (x in...)  loop needing to be enclosed in an if20:00
hatchthat's crazy I've never heard of that before20:00
hatchthenagain me and that linter don't agree on a lot of fronts :D20:01
baci think it is to force you to use a hasOwnProperty test?  didn't seem appropriate here20:01
hatchahh I suppose that's valid20:01
bacthe if (true) was used elsewhere so i stole it20:01
hatchI guess you could also switch to a YUI iterator of sorts20:01
rick_h_I broke jenkins. Does anyone have access to give it a boot? gary_poster ran it again earlier with success on my first branch today.20:02
gary_posterrick_h_, on it20:04
rick_h_gary_poster: ty much20:04
hatchrick_h_ yep20:04
hatchoh20:04
rick_h_sorry to keep bringing it down :(20:04
hatchnm :)20:04
gary_posterrick_h_, not your fault :-(20:04
* rick_h_ wonders why jenkins hates him so...goes to buy flowers20:04
hatchgary_poster: do I have your blessing this week to look into cleaning up the tests for an afternoon to try and solve that issue?20:04
hatch:)20:05
gary_posteron call20:05
hatchanyone else need any reviews before I get back to doing real work? ;)20:06
benjiMakyo: I'm reviewing your branch.20:22
Makyobenji, \o/20:23
MakyoI'll back out the config-debug changes, forgot to revert that.20:23
hatchis there a way to get a service model from a unit name?20:24
hatchor do I need to split on '/' ?20:24
gary_posterhatch, re blessing: if the in-browser stuff is nearing completion--only a few "implement X" cards left--or if you are blocked, yes20:32
hatchalright I'll keep that in my back pocket20:32
gary_postercool20:32
* gary_poster goes to reviews20:32
gary_posterbac, hatch, very open to hatch's idea of a separate closure.  I'd like that to be a separate, follow-on branch if that's what you decide to do20:34
bacok20:34
gary_posterunless it is done already, of course.  my point is merely that if this works, it unblocks other efforts, and is a great incremental step forward, so let's land it20:35
bacMakyo: two reviews done on your branch20:39
bacgary_poster: hatch's suggestion has not been done yet20:40
bacbcsaller: great suggestions.  thanks.20:42
bcsallerbac: :) glad to help20:42
bacgary_poster: could you please review ben's comments and leave a comment on the MP as how you'd like me to proceed?20:47
gary_posterok20:47
gary_posterbac, how long will it take20:47
bacprobably 1/2 day20:49
gary_posterbac, not too bad.  up to you but I suggest that you see if you can identify a compromise with reviewers to land this today (or early tomorrow) and then make a new branch tomorrow.  ben's db stuff in particular is the right thing to do technically, though not necessary at this instance, bot if it is half a day and you land incrementally I'm ok with it20:52
gary_posterbut20:52
bacbcsaller, hatch: you ok with that?  land the branch basically as is with a follow on fix?20:54
hatchyeah sure - I'll LGTM it if you can be sure you destroy that stuff in the test please :)20:55
bcsallerbac: I'm ok with that plan as well, if another card is made. 21:00
bcsallerI'll update the MP21:00
bcsalleror CR, whatever :)21:00
bacthanks21:00
bachatch: your comment about cleaning up in the tests is directed at the creation of a charm.  but the charm doesn't have any attached events as your comments states.  so to be clear, you are requesting the service with the events be destroyed or the charm too?21:10
bachatch: it's not enough to destroy the env and app?21:10
hatchbac: I may have misread - but I thought it was attaching events to something that wasn't being destroyed21:11
hatchif that's not the case then feel free to disregard21:11
hatchlet me check21:12
bachatch: the events are attached to the services modellist.21:12
baclord knows i don't want to introduce any extra test time bombs21:12
hatchbac: ok looks like the instance I was concerned about is actually attaching an event to itself so if you charm.destroy(); every time you `new models.Charm();` then that will be good21:16
hatchI figure that anytime we use `new` we should follow it up with a `destroy()`21:17
hatchI'm hoping that will help :D21:17
hatchgary_poster: any idea how I can test the is_subordinate code?21:30
hatchsince we dont' have a connection to the real charm store21:30
gary_posterhatch not sure what you mean--sorry a lot of plates going.  context?21:30
hatchthe remove_unit fake back end code now throws errors if the service is_subordinate21:31
hatchto match the improv code21:31
hatchbut how do I create a subordinate service21:31
hatchI guess I could create another dataset in test/data with a subordinate charm?21:37
gary_posterhatch just flip the flag on the service?21:37
gary_posterin a test setup?21:37
hatchoh heh...I never thought of that21:37
* hatch slowly walks away21:37
gary_poster:-)21:38
Makyobenji, I know it's late, but are you still around?21:58
benjiMakyo: just barely21:58
Makyobenji, quick question, feel free to tell me off.  Which yuidoc comment style do you prefer?  I used one from the yuidoc docs, but can easily change.21:59
benjiI don't really care, we started with the ones with *s down the side then some started removing the stars and one of the spaces (leaving the text 2-space indented from the opening "/**"22:00
benjiyours have /**, no indent and then **/  (as opposed to "*/") plus I'm sure there is one or two other variations22:00
hatchIn need of 2x reviews please https://codereview.appspot.com/8280044/22:01
benjiso my comment was more of a "we need to figure out what we want and enforce it" than a "you're doing it wrong! die in a fire!" ;)22:01
Makyobenji, alright.  Deindented while trying to remove the star, will hunt around for the most consistent.  And yeah, no fires :)22:01
hatchI do /** */ then with a 2space indent22:01
hatchwith no stars down the side22:02
MakyoAlright.22:02
benji^^^ would be a fine prescription as far as I am concerned 22:02
hatchMakyo: you can see how I do it while you're reviewing my branch :P22:02
hatchhaha22:02
benji(and I'll even try to get around to updating the yuidoc linter to complain on anything else)22:02
MakyoWell played.  I'll review it just for that :)22:03
hatchlol ^522:03
hatchjujugui anyone else available for a review before EOD?22:15
benjihatch: I can get you real quick.22:15
benjihttps://codereview.appspot.com/8280044/ right?22:15
hatchgreat thanks22:15
hatchyep22:15
benjihatch: done22:22
gary_posterhatch I will look before I stop today22:23
hatchgary_poster: no need, both done :)22:23
hatchthanks though22:23
gary_posterbut if you have other reviews and feel good about it land22:23
gary_postercool22:23
hatchnow onto expose and unexpose22:27
hatchoh no first I'll clean up those tests with my new utility methods22:28
hatchhey look at that, CI test caught a real failure22:44
hatchlol22:44
* hatch is fixing22:45
hatchhmm unable to reproduce failure on my local machine22:54
hatchwill fire off another CI see if it was a fluke22:54
hatchhmm the test happens on prod on IE22:56
hatchinvestigating22:56
hatch*sigh*23:00
hatchlooks like a bad merge into trunk23:02
hatchOR not23:02
hatchI love when different tests fail every time you run them23:08
hatchhmm it's failing on revno 48723:17
hatchgoing to keep stepping back23:17
hatchanyone else around still with a win 8 vm?23:26
hatchok somewhere between 483 and 486 breaks IE23:33
hatchsorry its taking me a while to build each version....slow internets23:33
hatchok some change http://bazaar.launchpad.net/~juju-gui/juju-gui/trunk/revision/486 causes tests to fail in IE all over the place23:43
hatchok it's in test_charm_container.js23:49
hatchtest 'only shows items up to the cutoff at first' causes it to fail...23:54
hatchoh rofl wow - I can't believe it23:56
hatchok I'll patch this tonight23:56
rick_h_hatch: we brokage things on you? 23:57
rick_h_hatch: heh, yea we talked about getting a Windows/IE10 setup going for everyone today actually. That sucks23:58
hatchrick_h_: well....yes you 'did' but it's not your's or jc's fault23:58
hatchthe changes to CharmContainer somehow exposed an issue that we were destroying the container and THEN destroying the CharmContainer instance causing it to throw errors when it tried to destroy the container23:59

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