[01:51] I think I did all of the reviews that were needed [01:51] if anyone needs one tonight let me know === rogpeppe1 is now known as rogpeppe [10:59] frankban: ping [10:59] rogpeppe: pong [11:00] frankban: i was just looking at statecmd.ServiceGet and wondering if you used the Charm field in the ServiceGetResults [11:00] frankban: i was wondering if it should really be CharmURL [11:01] frankban: as we're using that as a key [11:01] rogpeppe: looking in the GUI code [11:01] frankban: thanks [11:05] rogpeppe: AFAICT, we only use config and constraints in the callback [11:05] frankban: ok, cool, thanks [12:27] thanks for the review frankban [12:27] frankban, rogpeppe, bac, we use service.charm from ServiceGet in the app/store/emdpoints.js code [12:28] gary_poster: ok. i had decided not to change it now anyway [12:28] cool rogpeppe [12:28] gary_poster: but in general, the charm name is fairly useless - the charm url tells us everything (including the charm name, 'cos it's easy to get metadata given the url) [12:29] rogpeppe, oh! [12:29] rogpeppe, yeah I may have misunderstood. We need the charm url as the id for this stull [12:30] gary_poster: ah. currently ServiceGet returns the charm name, not the url [12:30] :-/ [12:30] gary_poster: it looked wrong to me. [12:30] gary_poster: i'd prefer to change the field to CharmURL to match the other places, but that would affect your code. [12:30] bac, can you weigh in on above? I'll try to finish my review of your branch from yesterday (sorry) in return [12:30] gary_poster: I cannot find any call to get_service in that file [12:31] frankban: it is added in my new branch [12:31] ah, that's where I saw it :-) [12:33] rogpeppe: so the proposal is to replace the charm name with the charm url in Service Get? we'd then have to do another query to get the charm metadata in order to find the name? [12:33] gary_poster, bac: but AFAICT, Brad's charm does not change the fact that we don't use the returned Charm in the go env [12:33] bac: i'm presuming you need the charm metadata around anyway [12:33] * gary_poster defers to bac [12:33] sorry, my irc was off-line and i didn't notice, so i'm catching up [12:33] bac: so you'll be maintaining a cache of charm metadata that you can query at any time, keyed by charm url [12:34] bac: why do you need the charm name, BTW? [12:43] bac, LGTM with comments [12:43] gary_poster: thanks [12:44] rogpeppe, gary_poster, frankban: upon looking at the code it looks like we don't use the charm name returned from get_service, nor would we use the charm URL if it were there. the charm is loaded in the processing of the delta stream. [12:44] bac: cool [12:44] frankban: is that what you understand? [12:44] bac, oh interesting. that means we are waiting for the delta stram [12:45] gary_poster: unless i'm missing something [12:45] when ideally we could use the immediate result [12:45] bac, so my take away is that if we added CharmURL that would make the GUI more responsive [12:46] is that reasonable? [12:46] gary_poster: if you look at loadService, the callback for get_service for all environments, only constraints and config are used from the results [12:48] gary_poster: perhaps it is reasonable. what is the latency between us recognizing we have a new service and calling get_service vs. waiting on the delta? not rhetorical, i don't know as i haven't messed with the watcher code. [12:52] bac, gotcha. My question is whether we could make the GUI more responsive in the new code. I guess you are right: this would only help for someone who went directly to a settings url of a service, perhaps (rather than going to an environment, waiting for a delta, and then clicking on the service), and maybe even that is not pertinent. [12:52] I think it is pertinent, but it is not a terribly important use case atm [12:53] gary_poster: it may be a win, i'm just saying i can't say right now. [12:53] ack bac [12:53] let's not worry about it atm [12:54] maybe document it somewhere for future work. seems very low priority now. [12:56] frankban, CSS fix LGTM, and I suggest/request landing right away [12:57] gary_poster: thanks, landing [13:00] bac: just saw the back scroll: yes, that's what I was saying. We do not store the charm url in the get_service callback. That URL arrives from the delta. [13:05] frankban: thanks [13:06] * frankban -> late lunch [13:23] weird, I could not convince Chrome to reload the stylesheet [13:24] the only thing that worked was a forced reload directly on /juju-ui/assets/juju-gui.css [13:44] ugh! fix CI! Someone! Please! [13:44] * gary_poster will look when possible [14:10] bac has your branch 'fix settings link' landed yet? [14:11] hatch: no [14:11] hatch: working on post-review changes [14:11] ok I just wanted to know it's considered bad practice for anyone to have access to the app [14:11] s/know/say [14:12] as it makes things way to tightly coupled [14:12] bac, hatch, agree as a rule. may be exceptions especially to deal with existing code [14:13] in that case happy to regard that kind of thing (passing app) as low priority bug to be fixed when possible [14:13] hatch: how would you restructure it? [14:14] bac, he means just like with the app show_environment [14:14] bac, you pass what you need [14:14] which may be quite a lot [14:15] yeah the idea is that the application is a controller [14:15] which starts to call the advice into question IMO [14:15] even though I agree with it largely [14:15] so you would never pass a controller around an app [14:15] i need the app for the callback loadService [14:15] you fire events to tell the controller what to do [14:15] bac, loadService: Y.bind(this.loadService, this), [14:16] well there is that [14:16] :) [14:16] this damn loadService function [14:16] haha [14:16] maybe would be interesting to have a state object or something [14:16] that app creates [14:16] I just removed it from being used in my branch [14:16] not high on my list to consider atm [14:17] (a separate state object I mean) [14:17] typically the application would listen for an event and then fire the loadService method [14:17] s/fire/call [14:17] sounds reasonable [14:18] then you will have access to the proper context to execute the loadService method under [14:18] break those coupled chains! [14:18] * hatch thinks that could be a pop song [14:19] ♫ break those coupled chains......♫ [14:22] rick_h_: has your branch that has the new way of doing navigation landed? I looked at the log and didn't see anything that looked like it. [14:22] hatch: at your EOD you asked about reviews to do. do check the kanban board reviews lane if you're looking, especially when other folks aren't around to answer! [14:22] Blackbird singing in the dead of night [14:22] Break those coupled chains and learn to fly [14:22] All your life [14:22] You were only waiting for this moment to arise [14:22] benji: no, still writing tests. Tryingn to get done asap today [14:22] bac: yep I did everything that was in that column [14:22] but sometimes people forget to drag it [14:22] hatch: except mine... [14:22] ok, thanks [14:22] it was there! i swear! [14:22] bac: it had juju-core and gary [14:22] so I thought it didn't need mine [14:22] :) [14:23] hatch: juju-core isn't a reviewer [14:23] juju-core is just a tag that means it has something to do with the go side, even it is a pure GUI fix [14:23] ohh woops I missunderstood [14:23] no biggie [14:23] I thought that meant it was held for a core guy [14:37] Bit of an experiment: http://jujugui-code-docs.s3-website-us-east-1.amazonaws.com/ [14:38] Nice Makyo [14:38] cool so now we can really see where we are missing documentation :D [14:39] Pretty easy to do, too. s3cmd sync yuidoc/ s3://jujugui-code-docs [14:39] Makyo: cool [14:39] cool - at some point we could set up a doc server for that [14:40] it could pull the latest docs from the source control [14:40] Makyo, nice [14:40] hatch, you can also see them using "make view-code-doc" [14:41] yeah but that's not nearly as cool [14:41] :P [14:41] Haha [14:41] Could have a make target for this with code-doc as a dependency. [14:41] Would need to figure out perms on the bucket, though. [14:42] gary_poster: it seems it is possible to always dupe the three errors in firefox, just running the tests and then clicking on another window (i.e. firefox must not be the active window) [14:42] it could be on the same server that's running the CI :) [14:42] frankban: when I do that I get more than the 2 failing errors [14:42] frankban, ah! even in devel mode? [14:43] usually 6+ [14:43] will be tough to debug that way, maybe [14:44] hatch: make test-prod-server, point firefox to http://localhost:8084/test/index.html and then click away: I see 3 errors, and they seems to be the ones are destroying our CI system [14:45] frankban: yeah when I do that it takes over 5 min to run the tests and 6+ fail [14:45] WAT [14:45] firefox is 9x slower to run the tests than chrome [14:45] yeah I have no diea [14:45] idea even [14:48] guihelp, can't find docs on how to manage those xxx-cert.pem and xxx-private-key.pem files in .juju [14:48] hatch: so on my machine is's 27s chrome devel and 40s FF nightly (4 fails...) [14:48] rick_h_: yeah could be that I'm using OSX [14:48] I can give er a go on my laptop [14:48] hatch: wonder if nightly has updates that'll make it faster soon for you as well [14:48] problem is, when I run "juju status" i get "error: The specified key does not exist." [14:48] hatch: just 9x seems drastic [14:48] not sure it's related [14:49] the access-key and secret-key settings in environments.yaml are correct [14:51] yay icons http://uistage.jujucharms.com:8080/bws/sidebar/precise/mysql-16/ [14:52] oh rick_h_ I found out what was causing the `Uncaught Error: InvalidStateError: DOM Exception 11` error [14:52] it wasn't DOM related at all [14:52] it's trying ot make a ws call before it's connected [14:52] hatch: ah, gotta love new tech :) [14:53] hatch: it seems that sending the window back is breaking all the focus/blur tests. it's weird because selenium should always have the windows active [14:54] jujugui please update kanban [14:54] frankban: ahhhh that would make sense I suppose....but those tests aren't using selenium [14:54] they are using js simulate [14:54] (although it should still be in focus) [14:57] hatch: going to hit you up for review if you've got a sec. I need to get this routing branch stuff landed asap please :) [14:58] hatch: I meant: our CI is using selenium, and there the window should always be focused [14:58] jujugui call in 2 [14:58] hatch: http://yuilibrary.com/projects/yui3/ticket/2529211 [14:58] frankban: I wonder if it's a YUI bug, maybe try writing a simulated event in raw js [14:58] rofl 2010 [14:58] closed because expired [14:58] hatch: https://codereview.appspot.com/8726048 jcsackett in case you're interseted in the changes/search stuff I mentioned [15:02] rick_h_: dig. you need an actual review, or is this just for me to look? [15:04] jcsackett: getting hatch and sinzui for reviews so just fyi [15:04] rick_h_: awesome. much easier to just look at the bits that touch my code. :-) [15:51] [15:51] rick_h_: ok I'll do that review now [15:51] hatch: k, sec. Pushing up a new rev per comments from sinzui [15:51] alright lemme know [15:52] once the lbox dance complorts [15:53] curses! failed re-google-auth [15:54] benji: And what was the problem the checklist procedure revealed? “The hospital was out of artificial knees,” [15:54] rofl [15:55] :) [15:55] hatch: https://codereview.appspot.com/8726048 ok up [15:56] there are several good checklist studies out there, one showed infection rates go down dramatically after implementing checklists that listed things like "wash hands" and "apply disinfectant to wound" [15:56] it just goes to show that humans are bad robots, and sometimes we need robots [15:56] benji: did you want to chat on the event stuff or you ok? [15:56] interesting [15:56] rick_h_: if you have a second I could show you the two issues I'm having at the moment [15:57] rick_h_: holy moly that's a diff [15:57] rick_h_: the regular hangout is open [15:57] benji: cool, omw [15:59] promises today! [16:30] rick_h_: review done [16:30] hatch: thanks, going to break from lunch and I'll clean up and such in a bit. [16:30] benji: so I've got 2 +1's if you're willing to pull this branch in and use the events for that [16:30] benji: and then I can race you to lbox after lunch [16:32] rick_h_: cool [16:33] anyone fancy a review? https://codereview.appspot.com/8702045 [16:33] * bac lunches [16:34] bac, looking [16:38] bac: can you repropose [16:38] the diffs don't work [16:39] yeah, "error: old chunk mismatch" [16:40] the diff on launchpad is ok though [16:40] yeah - I find those hard to read [16:42] I'm pretty sure it's a LGTM [16:43] it's been LGTM'd [16:43] we need a word for a successful review [16:43] LoGToM ? [16:44] "yeah that's been LoGToMed" [16:44] loogootoomee [16:44] lol I like it! [16:45] it sounds suspiciously close to lobotomy, though [16:46] sometimes that's what reviews feel like too though [16:46] haha [16:46] (granted not with this group) [17:11] hatch: thanks, updated per your notes and handing off to submit yay yay yay! [17:11] ^5 [17:19] jcsackett: benji branch has been accepted yay [17:20] cool [17:24] teknico, hatch: thanks for the reviews despite the technical difficulties [17:26] I wonder what causes that [17:26] it's happened a couple times to me too [17:50] did we decide to tag UI/UX bugs or not? [17:50] that was for rick's stuff [17:51] benji: if it's charmbrowser there's a tag for that [17:51] right [17:51] it's not, so I'll leave it [17:51] benji: and having UX folks tag their bugs 'charmbrowser' but if it's generic there's not a set tag atm [17:52] I think this bug should be "high" so I made it so, but it is a low sort of high: https://bugs.launchpad.net/juju-gui/+bug/1170782 [17:52] * benji takes lunch. [17:56] * gary_poster goes to camp. have a great weekend! [17:56] you too! [17:56] Have fun [17:56] * hatch wants to camp [18:01] does anyone here buy tv season passes through itunes? [18:02] evil apple :P [18:02] but no [18:02] rick_h_: can you chat for a sec? [18:02] jcsackett: sure thing [18:03] rick_h_: I know, but I pay like $100/mo for cable which could easily be cut in half by just buying the shows on itunes [18:06] hatch: yea, we cut a while ago going roku + AMZ. just giving you a hard time [18:06] ahh yea - I wish we could get that stuff here [18:06] we are pretty much left with itunes and netflix [18:07] hatch: gotcha [18:07] so not bought seasons on itunes, but done it on AMZ/roku. It's how I keep up on big bang theory [18:09] one of these days the fatcats will realize that borders are for sissies and accept our funnymoney [18:30] cool, staging updated so links are working yay http://uistage.jujucharms.com:8080/bws/fullscreen/precise/mysql-16 [18:30] just have the one issue with the /sidebar to /fullscreen [18:31] toggle back to fullscreen and sidebar...over...and over...and... [18:35] benji, would you be willing to QA/review https://codereview.appspot.com/8872044/ to see if that fixes #1170782 that you filed earlier? If not, no worries. [18:35] That's lp:~makyo/juju-gui/resize-breaks-panel FWIW [18:35] Makyo: I can in a few minutes (after lunch) [18:35] benji, sure. Lunching now, too. [19:08] Makyo: that did fix the problem, but it introduced scroll bars when the config panel is displayed [19:11] benji, oh, hm. I'm seeing a scrollbar in uistage, too. It looks like something's being pushed off the right side by a pixel or two, and the horiz. scrollbar is introducing the vertical one. [19:14] rick_h_: looks like it's coming together nicely [19:26] Makyo: your last revision fixed the scrollbars for me; reviewing the code now [19:26] benji, thanks, was just about to ping. [19:27] Border width adds to overall width, which wasn't being taken into account in one place. [19:39] Going to run to a coffee shop and leave the dogs home alone for a while. Back in a few. [19:43] i'm looking at the jenkins problem... [19:44] man jenkins is slow to respond [19:46] bac: you mean the firefox errors? [19:46] hatch: just looking at it and trying to figure out jenkins. do you know anything about the failures? [19:47] yeah the two failures are because firefox is not in focus so the click simulations don't work [19:47] http://yuilibrary.com/projects/yui3/ticket/2529211 [19:47] so we need to figure out why 1) this just started happening 2) how to solve it :D [19:48] hatch: ok, that's good to know [19:48] yeah that's all I know, and that's as far as frankban got resolving it [19:48] at least as far as I know of [19:48] I know know because of his work :) [19:48] hatch: oh, that's the bug that just expired b/c everyone was scared of it? [19:49] well if you read through lukes comments it looks like there isn't a way to fix it without a kludge [20:09] Makyo: 2nd review done [20:09] bac, cheers, thanks. [20:18] rick_h_: I'm trying to figure out the best way to pass the deploy function int the browser subapp, but I am in a twisty maze of passages, all alike. [20:20] s/int/into/ [20:21] service views converted to use promises [20:21] dramatically cleans up the code [20:38] A good idea would have been to bring a charger with me. Sigh. Back in a bit. [20:57] benji so maybe hatch can suggest thr best place for the dubspp to hook up to an event with the main app? [20:58] benji id prefer we fire up an event that the app is expecting to do the deploy [20:58] rick_h_: oh, I meant to say here that I figured it out [20:58] right now I'm trying to figure out why addeing a console log makes things work properly [20:58] shrodingers promise [20:58] heh [20:58] sorry tablet at coffe shop with my son so meh on typing [20:58] benji ah cool awesome then [20:59] that reminds me of a bug I had in termbeamer the other Saturday, if I put a pdb in a method then the code would work fine, if I took it out it would crash; it took me a couple hours to figure that one out [21:00] console.log("template done"); [21:00] container.setHTML(template); [21:00] console.log("container updated"); [21:00] container updated never shown but sometimes setHTML works [21:01] * hatch is so confused [21:02] this has to be a multi dispatch race condition [21:03] hatch welcome to my world.have to drop debuggers at the entry to eatch for each entery into esch function. [21:06] good news - if I make a syntax error it works every time [21:06] lol [21:29] rick_h_: I think console logs are async [21:29] and it's only an issue when you dispatch so many times [21:35] hah [21:36] woah and I think I figured out something crazy [21:37] need to do a couple more tries to see if this actually fixes it [21:37] well it didn't lol [21:38] but the issue is definitely that the showView is async and being called so many times [21:51] hatch: yea, might need to look closer at the idea of counting the requests/ditching the extra dispatches higher up [21:52] if (this._renderTimer) { [21:52] clearTimeout(this._renderTimer); [21:52] } [21:52] this._renderTimer = setTimeout(function() { [21:52] then showview is what I"m trying now [21:52] what a hack [21:53] looks to have solved the issue....however it's hardly a fix [21:53] but it's proof that the multi render is what's causing the issue I was having [21:56] ugh [21:56] yea, I know we keep beating around 'fixing' the issue but it really has sunk some serious dev time [21:56] yep [21:56] if it was easy anyone would have done it though I guess [21:57] yeah it's not 'hard' to do per-se' its just tedius because you have to go through each route and figure out what each dispatched method actually needs [21:58] and there will no doubt be side effects [21:58] heh [22:06] rick_h_: still around? [22:09] bcsaller: how about you? taken off for the weekend yet? [22:09] hatch: on my way out the door [22:09] hatch: what's up? [22:09] oh I'm just getting the multiple references to the same namespace [22:09] somehow an empty namespace is being tacked on the url [22:10] and was wondering if you solved that in your branch [22:10] heh, yea. I get that with my work but it 'worked' out. I just ignored it. [22:10] it gets kind of filtered everywhere I could tell [22:10] so seems like a warning I couldn't find a way around [22:10] I THINK it's causing the callback to be called twice [22:16] solved that [22:43] ahhhh [22:43] the router callbacks are being called BEFORE the url is updated causing it to be called again [23:23] yay only triggering the render once [23:23] finally! [23:23] still doesn't fix my problem lol [23:29] ahah! but I know what it is that's causing it...for some reason the onLogin callback is being called twice, once when it should, and once a good 5s after the env is done and ready [23:30] buuut I'm at a good stopping point - fixed the router code to dispatch/navigate propertly yay