| frankban | hi rogpeppe: I am fixing the branch, and I was thinking about the AuthName method you suggested. | 09:35 |
|---|---|---|
| rogpeppe | frankban: ok | 09:35 |
| frankban | rogpeppe: especially in relation to bug 1156715 | 09:35 |
| _mup_ | Bug #1156715: Replace EntityName() with something else. <juju-core:Triaged> <juju-gui:Triaged> < https://launchpad.net/bugs/1156715 > | 09:35 |
| rogpeppe | frankban: was it william's suggestion to report that bug? | 09:36 |
| rogpeppe | (out of interest) | 09:36 |
| frankban | rogpeppe: if we are going to replace EntityName with something better (e.g. GlobalName, APIName, ExternalName...), maybe it's worth making that change contextually | 09:36 |
| rogpeppe | frankban: i'm not sure | 09:37 |
| frankban | rogpeppe: I proposed to work on that once the annotations stuff is ready. IIRC, he suggested to find a better name for that method | 09:37 |
| rogpeppe | frankban: the problem is that now we've got two methods now: State.Entity and State.Authenticator, and we can't just assume that the names are the same for each | 09:41 |
| rogpeppe | frankban: ah, we've lost State.Entity, i see | 09:42 |
| frankban | rogpeppe: you mean State.Annotator and State.Authenticator | 09:42 |
| rogpeppe | frankban: yeah | 09:42 |
| rogpeppe | frankban: the problem is that the same "get name" method isn't necessarily appropriate for both | 09:43 |
| rogpeppe | fwereade: what are your thoughts here? | 09:44 |
| rogpeppe | fwereade: i'm suggesting that entities that can be returned from State.Authenticator have an AuthName method; and (if needed) entities that can be returned by State.Annotator have an AnnotatorName method. both methods would simply return the entity name as it is currently. i *think* this is what you might have suggested before. | 09:45 |
| fwereade | rogpeppe, +1 | 09:46 |
| fwereade | rogpeppe, I think it's better to have the two clear names for the two distinct contexts -- they can still directly share an implementation | 09:47 |
| frankban | rogpeppe, fwereade: ok. in this branch the AnnotatorName is not defined or required. We just include the EntityName() method in ad-hoc interfaced for tests. No problem in introducing an AuthName for authenticators, and no problem in closing that bug as invalid or wontfix. Sound good? | 09:49 |
| fwereade | frankban, I'd like to be consistent across Auth and Annotate, and ideally to drop the string EntityName across the board, but I'm not sure if that's a reasonable request in this context | 09:50 |
| fwereade | frankban, are there lots of clients using EntityName that aren't clearly asking in either an auth or an annotate context? | 09:51 |
| frankban | fwereade: I'd have to take a look, I don't think so, I guess EntityName is only used in the API context, and in the API we just have annotators and authenticators. Maybe rogpeppe can confirm. | 09:53 |
| rogpeppe | fwereade, frankban: actually EntityName is used in other contexts too | 09:54 |
| fwereade | frankban, hmm, I think it might be too big | 09:54 |
| rogpeppe | fwereade: in particular, it's used in the original way that it was created (to talk about a state entity in general, that might be a unit or a machine) | 09:55 |
| fwereade | frankban, rogpeppe: I think that if we're adding AuthName, though, I rather feel we should fix the instances of EntityName that are really AuthNames | 09:55 |
| rogpeppe | fwereade: sure | 09:55 |
| fwereade | rogpeppe, I may be changing my mind here | 09:56 |
| * rogpeppe still thinks that a single EntityName method makes sense (and saves us code) | 09:56 | |
| fwereade | rogpeppe, I think your motivation is sane but the name "EntityName" is not helpful | 09:57 |
| fwereade | rogpeppe, can we reconsider the list of possibly-sane alternatives? | 09:58 |
| rogpeppe | fwereade: ok | 09:58 |
| frankban | fwereade, rogpeppe: so, bug 1156715 is still valid | 09:58 |
| _mup_ | Bug #1156715: Replace EntityName() with something else. <juju-core:Triaged> <juju-gui:Triaged> < https://launchpad.net/bugs/1156715 > | 09:58 |
| fwereade | rogpeppe, I am feeling a gentle pull towards GlobalName but this opinion is not held strongly | 09:58 |
| rogpeppe | fwereade: i don't like GlobalName because it's not global - it's only "global" with respect to a given state. | 09:59 |
| rogpeppe | fwereade: and in the future it's quite possible we might have *real* global (including uuid) names | 09:59 |
| rogpeppe | fwereade: how about ExtName ? | 10:03 |
| fwereade | rogpeppe, true; hmm. LongName? :) | 10:03 |
| rogpeppe | fwereade: it's not too long, and abstract enough that it doesn't have any significant unwanted connotations | 10:04 |
| rogpeppe | fwereade: TaggedName ? | 10:04 |
| fwereade | rogpeppe, I could live with that :) | 10:04 |
| rogpeppe | fwereade: not that keen on LongName, i don't think | 10:05 |
| rogpeppe | nor on TaggedName really | 10:05 |
| fwereade | rogpeppe, fair enough, but ExtName is too unclear IMO | 10:05 |
| fwereade | rogpeppe, Nametag() :) | 10:06 |
| rogpeppe | fwereade: it's only unclear if it's documented unclearly IMHO | 10:06 |
| rogpeppe | fwereade: Tag() :-) | 10:06 |
| fwereade | rogpeppe, hum, yeah, that's not bad, is it? | 10:06 |
| rogpeppe | State.Authenticator(tag string) (Authenticator, error) | 10:07 |
| fwereade | rogpeppe, we'll want to fix a couple of things like DeployerName to DeployerTag but I think it works nicely | 10:07 |
| rogpeppe | fwereade: yeah, i could live with that | 10:08 |
| rogpeppe | type Tagger interface {Tag() string} | 10:08 |
| fwereade | rogpeppe, that SGTM, if you can live with it | 10:08 |
| frankban | fwereade, rogpeppe: so, should the Tagger interface be embedded in Authenticator and Annotator? | 10:09 |
| fwereade | frankban, yeah, it seems sensible I think | 10:09 |
| rogpeppe | frankban: i don't think so, otherwise you won't be able to do interface { Authenticator; Annotator } | 10:09 |
| fwereade | rogpeppe, oh, blast | 10:10 |
| fwereade | rogpeppe, well | 10:10 |
| frankban | rogpeppe: so TaggedAuthenticator, TaggedAnnotator? | 10:10 |
| rogpeppe | fwereade: but i have no problem with type TaggedAnnotator interface { Tagger; Annotator} | 10:10 |
| fwereade | rogpeppe, how often do we want both features in play at the same time? | 10:10 |
| rogpeppe | fwereade: i don't want to preclude i | 10:10 |
| rogpeppe | t | 10:10 |
| fwereade | frankban, rogpeppe: ok, fair enough | 10:11 |
| rogpeppe | fwereade: in general, other than at leaf level, you want either all-embedded or all-method interface types, i think. | 10:11 |
| fwereade | rogpeppe, yeah, sounds sane, ++composability | 10:12 |
| rogpeppe | fwereade: exactly | 10:12 |
| frankban | rogpeppe: i.e. in the API layer, we will always use Tagged(Authenticator|Annotator) | 10:12 |
| frankban | ? | 10:12 |
| rogpeppe | frankban: yes, and i think that the State.Authenticator should return TaggedAuthenticator | 10:12 |
| frankban | rogpeppe: cool | 10:13 |
| frankban | rogpeppe: and the same for State.Annotator i guess | 10:13 |
| rogpeppe | frankban: yup | 10:13 |
| frankban | rogpeppe, fwereade: what do you think about: 1) I land my current branch as is, with minor fixes (it's well over 1000 lines of diff) 2) I change that bug to be this Tag() and Tagger refactoring 3) I start working on that new bug, also replacing all the NamedAuthenticators and Annotators used in tests | 10:15 |
| fwereade | frankban, +1 | 10:16 |
| rogpeppe | frankban: i think that's a reasonable approach, except i'd like to see the entityName field go | 10:16 |
| rogpeppe | frankban: and i think that's an easier fix than making the code access it safely. | 10:17 |
| rogpeppe | frankban: (so it would use EntityName as i suggested in the review) | 10:17 |
| rogpeppe | frankban: and then it would change to use Tag in the later branch | 10:18 |
| frankban | rogpeppe: you mean in apiserver.go? so do you want me to temporarily add the EntityName to the Authenticator interface> | 10:18 |
| rogpeppe | frankban: i think that would be fine | 10:18 |
| frankban | rogpeppe, fwereade: thanks, I think we have a plan. | 10:18 |
| rogpeppe | frankban: wonderful | 10:19 |
| fwereade | frankban, sgtm | 10:22 |
| frankban | rogpeppe: could you confirm those notes are correct? https://bugs.launchpad.net/juju-gui/+bug/1156715/comments/1 | 10:51 |
| _mup_ | Bug #1156715: Replace EntityName() with something else. <juju-core:Triaged> <juju-gui:Triaged> < https://launchpad.net/bugs/1156715 > | 10:51 |
| rogpeppe | frankban: yup, seems good. there are other knock on effects of changing EntityName - various places use it and will need to change | 10:53 |
| frankban | rogpeppe: isn't enough s/EntityName/Tag for these cases? | 10:59 |
| rogpeppe | frankban: more or less - there are some variable and field names that will want renaming too | 11:00 |
| frankban | rogpeppe: maybe we'll need to take care of this case by case, with your help | 11:00 |
| rogpeppe | frankban: it should be fairly obvious, i hope - recursive grep -i entityname, and fix | 11:01 |
| frankban | rogpeppe: sounds good | 11:02 |
| frankban | rogpeppe: reproposed, would you like to take another look? | 11:19 |
| rogpeppe | frankban: will do | 11:19 |
| frankban | thanks | 11:21 |
| === teknico__ is now known as teknico | ||
| BradCrittenden | guihelp: in go, what is the relationship between packages and directories? in apiserver there is "package apiserver" and "package apiserver_test" and that is a-ok. if i add another package in that directory it complains. anyone here know? | 12:32 |
| gary_poster | I thought there was no connection, so obviously I'm no help | 12:33 |
| BradCrittenden | gah, my stupid nick | 12:33 |
| === BradCrittenden is now known as bac | ||
| === bcsaller__ is now known as bcsaller | ||
| === teknico_ is now known as teknico | ||
| hazmat | bac, if your adding another test i suggest reusing apiserver_test | 12:39 |
| hazmat | in general my understand is that its 1 to 1 directory containing src to pkg with dirname == pkg at least by convention, and that splitting out tests basically allows for stripping it from the binary and is specially handled by the test runner conventions. | 12:42 |
| bac | hazmat: i understand all of that from a hygiene perspective. i'm trying to understand the rules. it clearly isn't 1:1 so i'm confused why i cannot create a separate package, even if it is a bad idea. | 12:47 |
| hazmat | bac i think <package>_test is handled distinctly | 12:48 |
| hazmat | by the same convention that <name>_test.go is | 12:48 |
| bac | ok | 12:48 |
| hazmat | but that's guess work on my part.. | 12:50 |
| rogpeppe | teknico: did you run go test in your recent branch before submitting? | 13:03 |
| rogpeppe | teknico: because it broke trunk and i'm can't see how the tests could ever have passed... :-) | 13:04 |
| hazmat | rogpeppe, any comments on bac's question above re package naming | 13:04 |
| jcsackett | hatch: what sort of thing should i be looking for if tests pass under test-debug but fail under test-prod in the beforeAll with an apparent dependencies problem? | 13:05 |
| jcsackett | e.g. "extend failed; check dependencies." | 13:05 |
| jcsackett | i can't find anything missing in a requires, nor in the yui.use | 13:05 |
| rogpeppe | bac: hazmat is right. there can only be x and x_test in a given directory | 13:05 |
| rick_h_ | jcsackett: make sure you've pulled from trunk recently. | 13:05 |
| teknico | rogpeppe, I did run tests, and I plead guilty of insufficient scrutiny, because I did see failures that I interpreted as intermittent and unrelated | 13:05 |
| jcsackett | rick_h_: just merged, no difference. | 13:07 |
| teknico | rogpeppe, what's the overall tests status? are all of them supposed to pass all of the time, currently? | 13:07 |
| rick_h_ | jcsackett: make sure you make clean-all and rebuild as well. I guess it was a problem fixed on monday. | 13:07 |
| rogpeppe | teknico: yes | 13:07 |
| rick_h_ | jcsackett: and verify that tests pass on trunk if they still fail | 13:08 |
| rogpeppe | teknico: but with your branch as submitted, it cannot have passed the tests even once :-) | 13:08 |
| benji | the age-old story of untrusted tests; this is one of the reasons that tests need to be absolutely rock-solid | 13:08 |
| rick_h_ | jcsackett: I had a mess around tests like this yesterday I was going to bring up the on the stand up | 13:08 |
| rogpeppe | teknico: if you see a test failure, we want to know about it! | 13:08 |
| bac | thanks rogpeppe | 13:08 |
| teknico | rogpeppe, right, I will make a point to let you know :-) | 13:09 |
| teknico | (not about the failures in my code, though ;-) ) | 13:09 |
| teknico | or rather, in my changes | 13:10 |
| rogpeppe | teknico: BTW ISTM that both opClientAddRelation and opClientDestroyRelation are bogus, because neither of them undo the operation that they've performed. | 13:10 |
| teknico | rogpeppe, uhm. bac might be interested too ^^ | 13:10 |
| rogpeppe | teknico: i'm not sure when DestroyRelation came in. i should've been reviewing more branches, sorry! | 13:10 |
| teknico | rogpeppe, very recently too | 13:11 |
| bac | rogpeppe: oopsie | 13:11 |
| bac | rogpeppe: that one had lots of eyes on it. sorry you didn't get a chance to weigh in and catch that omission | 13:11 |
| teknico | it'll teach me landing changes right before lunch, ttyl :-) | 13:12 |
| rogpeppe | bac, teknico: in general, the func return from the op* functions is to reset the state back to the original. so if you make a successful change, you need to return a function to undo that change. | 13:12 |
| rogpeppe | i know it's awkward, and it would be nicer just to reset the whole thing each time, but unfortunately that's prohibitively slow. | 13:13 |
| teknico | rogpeppe, ISTR you mentioning that most op* functions could go away anyway | 13:13 |
| jcsackett | rick_h_: ok, i'll deal with email till standup. | 13:13 |
| rogpeppe | teknico: i discussed it with fwereade_ and he reckoned that it was worthwhile keeping them in | 13:13 |
| teknico | rogpeppe, good to know, thanks | 13:14 |
| hatch | jcsackett: get the problem fixed? | 14:00 |
| jcsackett | hatch: not yet. i'm double checking that the right modules are in my require | 14:00 |
| hatch | ok what you want to do is `make clean-all` `sh test-server.sh prod true` | 14:05 |
| hatch | then if you have dependency issues they will show up right away in those tests | 14:05 |
| hatch | frankban am I supposed to know what your email is about? Or did you sent it to the wrong person? :) | 14:07 |
| frankban | hatch: what email? | 14:08 |
| hatch | William and Roger agreed on using Tag(). .... | 14:08 |
| frankban | hatch: that's a comment on a bug | 14:09 |
| hatch | odd...it just came through as an email from you | 14:09 |
| hatch | heh | 14:09 |
| hatch | ok no problem :D | 14:09 |
| hatch | bcsaller: did you end up checking out my branch last night? | 14:11 |
| bcsaller | hatch: I did, but I haven't written anything up | 14:12 |
| hatch | oh ok - well we can chat in a hangout to save you the typing if you like | 14:12 |
| bcsaller | hatch: the way you assign _nsRouter to juju isn't how we typically do things though, you'd pass that down as an attribute. Its ok to attach constructors to the namespaces but usually not instances | 14:14 |
| hatch | Yeah I didn't like that either but that fn is being used in so many places that it's quite a lot of passing around for a single fn | 14:14 |
| hatch | I was almost thinking that we need a Y.juju.utils object | 14:15 |
| jcsackett | rick_h_: you see anything this file is missing for its require? http://bazaar.launchpad.net/~jcsackett/juju-gui/tab-widget/view/head:/app/widgets/tabview.js | 14:16 |
| jcsackett | rick_h_: this is the testfile, as well: http://bazaar.launchpad.net/~jcsackett/juju-gui/tab-widget/view/head:/test/test_tabview.js | 14:16 |
| === frankban_ is now known as frankban | ||
| hatch | jcsackett: does it actually say 'verrify dependencies' ? | 14:17 |
| jcsackett | hatch: Error: extend failed, verify dependencies | 14:17 |
| jcsackett | it's dying in the before() method in my test. | 14:17 |
| hatch | ok that means the Y.Base failed because it can't extend Y.Tab | 14:17 |
| hatch | Y.Base.create | 14:17 |
| hatch | there is no 'tab' module | 14:18 |
| jcsackett | hatch: i can't find any docs of where Y.Tab is defined--that's based on one blogpost i found about extending tab. | 14:18 |
| jcsackett | what should i include? | 14:18 |
| hatch | hmmmmmm | 14:19 |
| jcsackett | cause i can't find anything; it would be nice if TabView had a require, but alas. | 14:19 |
| hatch | ok I'm just getting to the tabview source | 14:20 |
| hatch | you need base-build and tabview | 14:24 |
| jcsackett | hatch: nope, same problem. | 14:24 |
| hatch | hehe _callbackMaybe 'call me maybe' ;) | 14:25 |
| jcsackett | hatch: that wasn't intentional, but once i realized the joke i became fond of the method. :-P | 14:26 |
| hatch | ok and does this happen only in the tests? | 14:26 |
| hatch | haha | 14:26 |
| gary_poster | jujugui call in 2 | 14:28 |
| jcsackett | hatch: i only get this in test-prod | 14:29 |
| * benji struggles with the Google Talk plugin again. | 14:29 | |
| * benji gives up and just reboots. | 14:30 | |
| hatch | ohh - whatabout make clean-all; make prod; ? | 14:31 |
| gary_poster | rick_h_ starting without you | 14:32 |
| rick_h_ | gary_poster: k, sec | 14:32 |
| hatch | jcsackett: the reason I'm hoping it happens with make prod is because then you can test to see if Y.Tab and Y.TabView are defined at the top of your closure | 14:40 |
| jcsackett | hatch: i'm not seeing anything. that could be because the tabview's not wired into anything else though. | 14:45 |
| hatch | hmm this sounds very odd | 14:45 |
| rick_h_ | hatch: do you have that nodejs bug handy from the npm issues? | 14:48 |
| rick_h_ | hatch: nvm, found it | 14:50 |
| hatch | I tweeted it | 14:58 |
| hatch | oh ok | 14:58 |
| jcsackett | hatch: could Y.Tab simply not be exposed in a way that it can be extended? | 15:01 |
| hatch | https://github.com/yui/yui3/blob/master/build/tabview/tabview-debug.js | 15:01 |
| hatch | there is the source | 15:01 |
| hatch | you'll see that it's exposed on Y.Tab | 15:01 |
| jcsackett | well, i suppose if that were the case my tests would die in test-debug as well. | 15:01 |
| hatch | ok here is what you can do | 15:01 |
| hatch | remove your tabview code from the app, open up the network tab in debug mode and look to see what files are being requested from yahooapis.com | 15:02 |
| hatch | then add it back in, and compare | 15:02 |
| hatch | our yui dep script doesn't catch all deps I've found | 15:02 |
| jcsackett | hatch: not sure what you mean by "open up the network tab in debug mode" | 15:02 |
| hatch | network tab in chrome = app built with make devel | 15:03 |
| hatch | s/=/0 | 15:03 |
| hatch | blah | 15:03 |
| hatch | I can't type | 15:03 |
| hatch | s/=/- | 15:03 |
| hatch | :) | 15:03 |
| hatch | basically you want to see if it's pulling in extra modules from yahooapis.com which it can't do under make prod | 15:03 |
| jcsackett | ah, ok. | 15:03 |
| hatch | FYI it pulls in some right now so you will need to compare | 15:04 |
| jcsackett | that will work even though tabview isn't wired in, so opening the app doesn't create any tabviews? | 15:04 |
| hatch | if it's in the 'use' then it'll pull in the code | 15:04 |
| hatch | I'm also assuming that you have linted this code :) | 15:09 |
| hatch | juuuuuuust to be sure ;) | 15:10 |
| jcsackett | hatch: yeah. | 15:10 |
| rick_h_ | jcsackett: is the latest pushed? | 15:11 |
| jcsackett | rick_h_: yes. | 15:11 |
| rick_h_ | I'll pull it down and try it out here localy | 15:11 |
| rick_h_ | jcsackett: so all tests pass for me. test-server and test-prod | 15:15 |
| jcsackett | ... | 15:15 |
| jcsackett | well, wtf. | 15:15 |
| rick_h_ | http://paste.mitechie.com/show/908/ | 15:16 |
| rick_h_ | ^^ is from test-prod | 15:16 |
| jcsackett | ima kill my branch and check it out again. | 15:16 |
| rick_h_ | jcsackett: yea, I just grabbed trunk, created a branch, merged yours in, and make test-prod | 15:16 |
| hatch | lol | 15:17 |
| hatch | man I hate it when that happens | 15:17 |
| jcsackett | this is maddening. | 15:20 |
| jcsackett | a completely clean checkout of mine fails. | 15:20 |
| jcsackett | evidently only on my machien. | 15:20 |
| jcsackett | *machine | 15:20 |
| jcsackett | welp, that's it. time to burn my computers and turn to a life of goatherding. | 15:21 |
| jcsackett | :-P | 15:21 |
| gary_poster | 2 review requested of https://codereview.appspot.com/7621048 | 15:21 |
| gary_poster | card is in high-priority maintenance review lane | 15:22 |
| hatch | gary_poster: I'll take one | 15:23 |
| gary_poster | ty | 15:23 |
| hatch | while I'm doing that - is there a way I can flatten commits with bzr? | 15:23 |
| Makyo | I'll take one. | 15:23 |
| hatch | aka like a rebase with git | 15:23 |
| gary_poster | hatch, rebase was frowned upon within bzr, because commits are hierarchical | 15:24 |
| gary_poster | I think there's a way to do it but I've never done it | 15:24 |
| hatch | ahh - I just did a number of small commits and looking at the branch now it's hard to tell what the diff is | 15:25 |
| hatch | no biggy I guess :) | 15:25 |
| gary_poster | hatch, instead you can simply branch trunk again, and then merge in your old branch | 15:25 |
| hatch | oh that will do one commit? | 15:25 |
| hatch | not merge all of the commits? | 15:25 |
| gary_poster | hatch, that will give you what you want, while maintaining history hierarchically | 15:25 |
| gary_poster | imagine a big commit, which, if you really look at it, can be subdivided into your old smaller commits | 15:26 |
| jcsackett | rick_h_: any notion what might be busted that would let it pass for you not for me? surely YUI require doesn't vary b/c of an lxc... | 15:26 |
| gary_poster | but the default view will be what you want | 15:26 |
| hatch | ahh ok perfect thanks | 15:26 |
| hatch | and a little OT - what | 15:26 |
| hatch | 's a spike? | 15:26 |
| hatch | you mention it in your branch | 15:26 |
| gary_poster | hatch, XP term (as if we do XP): http://www.extremeprogramming.org/rules/spike.html . Short translation: code that you write to figure something out and prove it works, and then expect to throw away | 15:27 |
| rick_h_ | jcsackett: not sure tbh. What's npm --version? | 15:28 |
| jcsackett | 1.2.11 | 15:28 |
| hatch | gary_poster: ohh ok - I just call those prototypes ;) | 15:28 |
| jcsackett | rick_h_: ^ | 15:29 |
| hatch | jcsackett: node version? | 15:29 |
| rick_h_ | jcsackett: yea, not sure. I'm .14 but as long as you're not newer you're ok | 15:29 |
| rick_h_ | there's a known issue with node 1.0 and the npm with it | 15:29 |
| gary_poster | hatch, +1 | 15:29 |
| jcsackett | hatch: v0.8.20 | 15:30 |
| hatch | hehe | 15:30 |
| rick_h_ | jcsackett: but at this point, I cna't say since it doesn't break for me. Only thing I can think is to try a new lxc from scratch and if it works a diff directory v directory | 15:30 |
| hatch | jcsackett: odd - after this review I'll also check it out to see | 15:30 |
| jcsackett | hatch: ok. ping me with results. i'm going to go have some lunch. | 15:34 |
| gary_poster | hatch, you'll be happy to know that I ran my tests on IE 10 as well :-) | 15:36 |
| gary_poster | (and they passed) | 15:37 |
| hatch | haha awesome | 15:37 |
| hatch | jcsackett: good news - make test-prod fails for your branch here as well | 15:47 |
| jcsackett | hatch: huh. so what's magical about rick_h_'s setup... | 15:50 |
| hatch | he probably lied and ran make test-debug :P | 15:50 |
| hatch | jcsackett: ok for some reason in your tests Y.Tab() isn't a constructor | 16:04 |
| hatch | neither is TabView for that matter | 16:07 |
| hatch | so you'll need to evaluate why there is no tabview on prod | 16:09 |
| rick_h_ | orly? I pasted my test results :P | 16:11 |
| hatch | rick_h_ was that make test-prod? | 16:12 |
| rick_h_ | hatch: yea | 16:12 |
| hatch | well how odd is that | 16:12 |
| rick_h_ | .zshhistory http://paste.mitechie.com/show/909/ | 16:12 |
| rick_h_ | :) | 16:12 |
| hatch | merge? | 16:13 |
| rick_h_ | yea, merged his changes into a local branch | 16:13 |
| hatch | oh - well that's not a clean checkout then is it? ;) | 16:13 |
| rick_h_ | oh...well no...but it is test-prod with success | 16:14 |
| hatch | lol | 16:15 |
| hatch | ok diff those two branches and we might find the missing link | 16:15 |
| rick_h_ | which two? This is jcsackett's branch. | 16:15 |
| hatch | whatever your local branch was | 16:16 |
| rick_h_ | http://paste.mitechie.com/show/910/ here, that's more complete. It was just trunk | 16:17 |
| hatch | ohh ok | 16:18 |
| hatch | I'll try merging into trunk | 16:18 |
| jcsackett | hatch, rick_h_: i did the same steps i see in rick_h_'s paste but no dice. | 16:20 |
| hatch | jcsackett: also no luck here | 16:21 |
| jcsackett | so rick_h_ is magical. | 16:21 |
| hatch | for whatever reason it doesn't appear that the tabview files are being loaded into the prod rollup | 16:22 |
| rick_h_ | lol, YUI auto loads to my commands because I rule | 16:22 |
| hatch | lol | 16:28 |
| hatch | jcsackett: i'd try to manually include the tabview files in the rollup | 16:30 |
| hatch | see if that solves the issue | 16:30 |
| hatch | see the merge-files script | 16:30 |
| jcsackett | hatch: ok. | 16:30 |
| hatch | sorry I'm just trying to do this namespace routing stuff at the same time | 16:31 |
| hatch | :) | 16:31 |
| jcsackett | hatch: success! | 16:35 |
| hatch | werd up! | 16:36 |
| hatch | what was it? | 16:36 |
| jcsackett | adding to merge-files | 16:36 |
| jcsackett | manually pushing 'tabview' into reqs fixed it. | 16:37 |
| hatch | ugh - that's the second time that script has failed us this week | 16:37 |
| jcsackett | and as several other modules are in there, i think this is not the first time someone has encountered this. | 16:37 |
| hatch | it's GOTA GO! | 16:37 |
| hatch | jcsackett: how I got to that point was to ack Y.TabView in the rollup | 16:39 |
| hatch | and when it wasn't found that was a big red flag | 16:39 |
| jcsackett | hatch: dig. i'll remember that if i hit this again. | 16:40 |
| jcsackett | hatch, rick_h_: can you fine folks take a look at https://codereview.appspot.com/7663050 when you have a moment and see what else might be wrong with this code? :-P | 16:40 |
| rick_h_ | jcsackett: what's wrong now? | 16:41 |
| rick_h_ | jcsackett: or you just mean general review | 16:41 |
| jcsackett | rick_h_: the latter. i was just being self-deprecating. | 16:42 |
| jcsackett | which, admittedly, is misleading in this instance. | 16:42 |
| rick_h_ | jcsackett: heh, ok will peek in a minute | 16:42 |
| rick_h_ | jcsackett: yea, nothing to get down about. I spent half of yesterday debugging stuff just as well | 16:42 |
| rick_h_ | poor hatch gets brought into them with no hope :) | 16:42 |
| jcsackett | rick_h_: not down at all. quite pleased in fact to have sorted it. :-) | 16:42 |
| jcsackett | as long as i'm eventually getting stuff done, i'm happy/ | 16:43 |
| rick_h_ | jcsackett: heh, and I think I see your issue. http://yuilibrary.com/yui/docs/tabview/ | 16:43 |
| rick_h_ | you used a name already in the YUI namespace | 16:44 |
| rick_h_ | nvm, I guess it is 'browser-tabview' | 16:44 |
| hatch | one of these days I'll have to look into that script | 16:44 |
| hatch | to see why it misses things | 16:44 |
| jcsackett | yeah, the file is the same, but the ns is different because our browser prefix. | 16:44 |
| rick_h_ | jcsackett: right | 16:45 |
| hatch | review done | 16:47 |
| hatch | now if I can get back to MY OWN WORK!! | 16:47 |
| hatch | :P | 16:47 |
| hatch | lol | 16:47 |
| benji | "16 conflicts encountered." This branch may never end. I always thought my life's work would be something more substantial than adding a single command to an API. | 16:47 |
| rogpeppe | gary_poster: ping | 16:47 |
| gary_poster | hey rogpeppe | 16:48 |
| hatch | benji: lol | 16:48 |
| benji | :) | 16:48 |
| rogpeppe | gary_poster: so... the most significant allWatcher branch has just landed. | 16:48 |
| rogpeppe | gary_poster: and you *might* be able to see something actually working | 16:48 |
| rogpeppe | gary_poster: i wondered if you wanted a chat about where we want to go from here | 16:49 |
| gary_poster | rogpeppe, I saw the one earlier today that had the machinery. now we have the one that actually hooks it up too? | 16:49 |
| rogpeppe | gary_poster: yup | 16:49 |
| gary_poster | awesome! | 16:49 |
| benji | yay! | 16:49 |
| rogpeppe | gary_poster: i'll believe it when i see the black triangle :-) | 16:50 |
| gary_poster | rogpeppe, sure, we can chat. http://tinyurl.com/guichat | 16:50 |
| rogpeppe | gary_poster: i'm there... | 16:51 |
| rick_h_ | jcsackett: review heading your way. I think there might be some more to clear up though on the way it works | 16:57 |
| rick_h_ | honestly, now that I look closely I think a tabview that allows vertical was all we need. | 16:57 |
| rick_h_ | jcsackett: sec, heading home from coffee shop and we can do a hangout in 10 | 16:58 |
| hatch | bcsaller: is the best way to get data into topology/service.js to pass the options in via the addModule() call in views/environment.js ? | 16:59 |
| bcsaller | hatch: we typically pass data to the env view and then from that to the topology. All modules will have access to their topology. | 17:00 |
| === deryck is now known as deryck[lunch] | ||
| hatch | gotcha | 17:03 |
| hatch | damn you dependency injection | 17:03 |
| hatch | global variables for everyone! | 17:03 |
| gary_poster | benji, quick call in guichat? | 17:19 |
| gary_poster | benji, re https://code.launchpad.net/~gary/juju-gui/megawatcher | 17:20 |
| gary_poster | you are probably lunching, as I should be | 17:20 |
| hatch | lunch smunch | 17:20 |
| gary_poster | :-) | 17:20 |
| hatch | bcsaller: ok I pushed my changes to use dep injection, now I'm still trying to track down why this test is failing | 17:21 |
| hatch | I REALLY wish mocha would stop squashing console.log | 17:22 |
| bcsaller | hatch: mocha or the console handler in app? on a prod build the console is off by default | 17:23 |
| bcsaller | mocha shouldn't touch it | 17:23 |
| gary_poster | hatch, we have a fake console now...I bet we could make the tests turn off the console (i.e., switch to the no-op console) if we are running in mocha | 17:24 |
| gary_poster | that way we can log whatever we want | 17:24 |
| hatch | bcsaller: well when you run sh test-server.sh debug true no console.logs() are output to the console | 17:24 |
| hatch | gary_poster: ahh | 17:25 |
| gary_poster | sounds like you are encountering a different issue than what I thought you were talking about though...maybe related | 17:25 |
| hatch | when you run the tests in the browser do you get any logs in the devtools? | 17:25 |
| gary_poster | I think so. looking (I usually use breakpoints instead) | 17:26 |
| hatch | odd - debugger; also doesn't trigger it to stop | 17:26 |
| hatch | for me | 17:26 |
| gary_poster | hatch I have lots of "loader: has Skin?" things | 17:27 |
| gary_poster | that's it | 17:27 |
| hatch | yeah I don't even get those in the tests | 17:27 |
| hatch | wierd!!! | 17:27 |
| gary_poster | huh, weird | 17:27 |
| hatch | weird even | 17:27 |
| hatch | :P | 17:27 |
| gary_poster | Maybe we turn our fake console on? Looking... | 17:28 |
| gary_poster | hatch, our console manager is set to noop for our tests somewhere...finding where | 17:31 |
| === matsubara is now known as matsubara-lunch | ||
| gary_poster | hatch, heh | 17:33 |
| gary_poster | I see why | 17:33 |
| gary_poster | we are running app tests | 17:33 |
| gary_poster | the app, when instantiated, defaults to turning the console off | 17:33 |
| gary_poster | unless you explicitly tell it to turn on | 17:34 |
| gary_poster | so hatch, the easy small solution is, in your test that you want to see console output in, call consoleManager.native() | 17:34 |
| gary_poster | (consoleManager is stuffed on window) | 17:35 |
| hatch | oh :) awesome thanks | 17:35 |
| gary_poster | to fix this generally...we'd have to do something in app to be more conditional about this somehow | 17:36 |
| gary_poster | we could set a global flag in the test file, for instance, and have app look for that | 17:36 |
| gary_poster | but anyway, that will get you going for now | 17:36 |
| hatch | yeah thanks - I just fixed the bug - but that will definitely speed me up :) | 17:37 |
| gary_poster | cool | 17:37 |
| hatch | well I fixed A bug :) | 17:37 |
| gary_poster | hatch if you want to contemplate better ideas, app/view/utils has the consoleManager and app.js uses it (just search for it) | 17:37 |
| hatch | sure I'll take a look at it | 17:38 |
| hatch | oy....102 failures | 17:39 |
| hatch | ok apparently a number of tests rely on routing | 17:42 |
| hatch | who knew!! | 17:42 |
| benji | gary_poster: hey, was lunching; call now? | 17:51 |
| gary_poster | benji, sure | 17:53 |
| benji | gary_poster: ahhhhh! Hangout problems; one minute | 17:54 |
| gary_poster | np | 17:54 |
| * benji reboots. | 17:54 | |
| * benji plays IT Crowd to get into the mood for "turning it off and on again" | 17:55 | |
| hatch | haha oh the IT Crowd | 17:55 |
| hatch | boo yeah tests pass | 18:13 |
| hatch | Could anyone familiar with the namespace routing code review https://codereview.appspot.com/7719052 please :) Thanks in advance | 18:19 |
| hatch | I'd also be intersted in a quick QA if possible | 18:19 |
| Makyo | Taking a look. | 18:19 |
| hatch | thanks | 18:20 |
| hatch | rick_h_ this latest branch will allow the url state to be maintained across the charmstore and gui however you'll notice that your views are now pushed offscreen | 18:22 |
| hatch | so that's just a markup/css issue to fix on your end once this lands | 18:22 |
| hatch | just FYI | 18:22 |
| rick_h_ | hatch: ok, thanks for the heads up | 18:23 |
| gary_poster | hatch I gave code review. About to try it, then will hopefully give a final stamp of approval | 18:26 |
| hatch | thanks - I was using the _ because that's how it was done elsewhere in the application | 18:27 |
| hatch | if it's alright i'd like to land with it then change after - as it'll need to be changed elsewhere in the application | 18:27 |
| gary_poster | hatch I know. But if we are passing it around everywhere, it definitely is not "protected" any more. change after: sure. Next branch though, pls. | 18:28 |
| hatch | yep will do | 18:28 |
| hatch | which story should I put that ticket under? | 18:29 |
| gary_poster | hatch, top | 18:31 |
| gary_poster | hatch, qa problem: | 18:31 |
| gary_poster | click on alerts | 18:31 |
| gary_poster | then "View All Notifications" | 18:31 |
| gary_poster | Takes you to http://localhost:8888/notifications/:gui:/service/haproxy/ | 18:31 |
| gary_poster | which does not work | 18:31 |
| gary_poster | sorry bad repro instructions | 18:32 |
| gary_poster | before all that, click on a unit or service | 18:32 |
| hatch | yep I missed that one, good catch | 18:33 |
| hatch | thanks | 18:33 |
| === deryck[lunch] is now known as deryck | ||
| gary_poster | hatch here's another one, from unit page (e.g. http://localhost:8888/:gui:/unit/memcached-0/) click on relation link. url goes to http://localhost:8888/:gui:/unit/memcached-0/mediawiki/relations/ and view shows environment (?!) | 18:43 |
| gary_poster | hatch one more | 18:43 |
| hatch | gary_poster: that's what it did before if I remember correctly | 18:43 |
| hatch | I thought it was intentional | 18:44 |
| hatch | do we have a relations view? :) | 18:44 |
| gary_poster | from service page (e.g. http://localhost:8888/:gui:/service/memcached/relations/) click on relations tab and then click on a link (like to mediawiki) | 18:44 |
| gary_poster | url changes to http://localhost:8888/service/mediawiki/:gui:/service/memcached/relations/ | 18:44 |
| gary_poster | and does not go anywhere at all | 18:45 |
| gary_poster | hatch we do have a relations tab, see above :-) | 18:45 |
| gary_poster | hatch, all of that behavior works properly in trunk/on uistage, so it is specific to your branch | 18:46 |
| hatch | oh I see :) | 18:46 |
| hatch | too....many.....links | 18:47 |
| hatch | :) | 18:47 |
| gary_poster | :-) | 18:47 |
| gary_poster | hatch, logout does not work on your branch afaict: takes me to an empty page | 18:48 |
| gary_poster | should take you to login | 18:48 |
| gary_poster | works in trunk | 18:48 |
| * hatch wonders if he got anything working properly | 18:49 | |
| hatch | ;) | 18:49 |
| gary_poster | hatch, that's all I can find. everything else is AOK | 18:50 |
| gary_poster | lemme know when you have addressed those and I'm happy to re-QA hatch | 18:50 |
| hatch | there is anything else? lol | 18:50 |
| gary_poster | :-P | 18:50 |
| === matsubara-lunch is now known as matsubara | ||
| hatch | nothing more frustrating than tests that only fail when they are run with the full suite | 19:20 |
| benji | yep; inter-test state dependencies are a major pain | 19:44 |
| hatch | yeah - the thing is that there shouldn't be any haha | 19:46 |
| hatch | anyone remember who wrote the notifier widget? | 19:58 |
| hatch | I'm trying to figure out why it's calling the notification view when there is no reference anywhere in the test or widget | 19:58 |
| bac | guihelp: dumb question: how do you clone a map in go? equivalent of python's newdict = dict(olddict) | 20:01 |
| Makyo | hatch, bcsaller and frankban, I believe. | 20:01 |
| bcsaller | hatch: my guess is its being subscribed in one of the app tests and then triggered later | 20:02 |
| hazmat | bac, iter old and set into new | 20:02 |
| bac | hazmat: oh. painful. | 20:03 |
| hazmat | its not too bad.. extra 3-4 lines. | 20:04 |
| hatch | bcsaller: it LOOKS like it's being rendered because of some type of event listener? | 20:04 |
| hatch | the traceback doesn't show much | 20:04 |
| hatch | ahah | 20:04 |
| hatch | that's exactly what's happening - now to find out where it's being instantiated at all | 20:05 |
| hatch | which may pose to be a bigger issue as it's probably one of the other tests hah | 20:05 |
| gary_poster | benji, yay on merge :-) | 20:25 |
| benji | :) | 20:25 |
| hatch | fixed! | 20:33 |
| hatch | Friday card created | 20:33 |
| hatch | lol | 20:33 |
| hatch | gary_poster: fixed code should be up - however I coudln't reproduce your logout issue - if you could try it again and let me know if it's fixed that would be appreciated :) | 21:13 |
| gary_poster | hatch, ok on it | 21:13 |
| hatch | thanks | 21:15 |
| gary_poster | hatch confirmed everything works now except logout. I'm using make prod: you try that? | 21:16 |
| gary_poster | I will try make devel now... | 21:16 |
| hatch | yeah works on both - dumps me at the login screen | 21:17 |
| gary_poster | hatch, duped on devel but I have more repro instructions: log out from inner page | 21:17 |
| gary_poster | hatch, verified that logout works from inner page on uistage | 21:18 |
| hatch | so an inner page like /:gui:/unit/memcached-0/ ? | 21:18 |
| gary_poster | hatch yes | 21:19 |
| gary_poster | hatch you are not seeing? If so will kill cache 1 sec | 21:19 |
| hatch | *closes as can't reproduce* | 21:19 |
| hatch | ;) | 21:19 |
| hatch | nope I honestly can't | 21:19 |
| hatch | it works everywhere I try it | 21:19 |
| gary_poster | hatch, just duped again after killing cache... | 21:20 |
| gary_poster | hatch screenshare to make sure we are on same page? | 21:20 |
| gary_poster | hatch, guichat | 21:20 |
| hatch | sure | 21:20 |
| hatch | gary_poster: it appears to be a linux issue | 21:29 |
| hatch | I can reproduce it there | 21:29 |
| hatch | well....linux chrome | 21:29 |
| hatch | ok there is some combination of linux chrome and prod which is causing the blank screen (yay at least it can now be reproduced) | 21:30 |
| hatch | on logout on prod it's destroying the application container | 21:57 |
| hatch | so it doesn't have anything to render into | 21:57 |
| gary_poster | glad you could dupe hatch | 22:42 |
| hatch | yep I've tracked it down to a single line | 22:43 |
| gary_poster | that sounds important for our audience | 22:43 |
| hatch | app.js ln 703 this.loggingOut is never true | 22:43 |
| gary_poster | huh | 22:43 |
| hatch | exactly | 22:43 |
| hatch | :) | 22:43 |
| hatch | if you put a debugger; statement on line 699 of app.js you can see that the login screen is actually rendered - then after check_user_credentials is called about 4 times it's gone | 22:45 |
| hatch | so that's how far I've gotten | 22:45 |
| hatch | ok it doesn't look like that's the cause | 22:47 |
| hatch | so yeah... | 22:47 |
| hatch | ok this.env.getCredentials() is null | 23:02 |
| gary_poster | it should be | 23:06 |
| hatch | fixed | 23:08 |
| hatch | I don't know WHY it broke but I fixed it | 23:09 |
| gary_poster | heh, hatch, you want me to look at it and bless it so you can land this thing and run away? | 23:10 |
| hatch | yup if you don't mind | 23:10 |
| gary_poster | on it | 23:10 |
| hatch | ok just lboxing now | 23:10 |
| gary_poster | oh ok lemme know | 23:10 |
| gary_poster | helping son with homework so in and out | 23:11 |
| hatch | sure no problem | 23:12 |
| hatch | annnnnd it's up | 23:14 |
| gary_poster | ok trying | 23:15 |
| * hatch crosses fingers | 23:17 | |
| gary_poster | hatch worked for me! crazy that this new else block is necessary in this one case. I will suggest that you comment it in the review, but otherwise LGTM. Thank you! | 23:18 |
| hatch | w0000000t!! | 23:18 |
| hatch | that one took forever to track down | 23:19 |
| hatch | glad that's done | 23:19 |
| hatch | well in reality it was only 30 minutes - why did it feel longer than that | 23:20 |
| hatch | lol | 23:20 |
| hatch | in this debugging however I found a number of performance improvements we can do in the future | 23:21 |
| gary_poster | hatch, cool, write em down in a card pls :-) . Meanwhile LGTM. Gave some trivial comments because wasn't quite sure why you opted for ../../ but probably good. it certainly works, so I suggest you just leave it | 23:29 |
| hatch | thank yas - well the place where those are located nested two deep from the parent context - I don't like it either but unless you know of a way to get the parent context I don't think we have any other option | 23:32 |
| hatch | for the record, I didn't like doing that either :) | 23:32 |
| hatch | I noticed you said / does that mean 'top context' ? | 23:33 |
| hatch | blarg - ok, that 'fix' that I did breaks other things in the app | 23:35 |
| hatch | wait no it didn't | 23:36 |
| hatch | O K it's time to call it a day haha | 23:37 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!