| gary_poster | argh, again | 12:10 |
|---|---|---|
| gary_poster | bac frankban sorry, lost track of time. I'm going to try and get video working. we'll talk in 5 minutes (12:15 UTC) one way or the other | 12:11 |
| bac | ok | 12:12 |
| gary_poster | gmb if you see this, please kick SpamapS about the charm packaging symlinks and python toolbox packaging | 12:18 |
| frankban | bac: sorry I've closed firefox, you were tellenig me something, I am back in goldenhorde | 12:28 |
| bac | frankban: ok, let's pair in about 10 minutes. i'll share a screen using benji's old method | 12:29 |
| frankban | ok | 12:29 |
| gary_poster | frankban: | 13:28 |
| gary_poster | <flacoste> gary_poster: nah, we always contribute patches upstream on their license terms | 13:28 |
| gary_poster | <flacoste> gary_poster: so this is fin | 13:28 |
| gary_poster | e | 13:28 |
| gary_poster | I will unblock your card :-) | 13:28 |
| frankban | cool gary_poster, so LGPL | 13:28 |
| gary_poster | right, frankban | 13:28 |
| bac | gary_poster: yay! | 14:09 |
| gary_poster | bac, ? | 14:10 |
| bac | gary_poster: frankban and i figured out what was going on. wadl generation caches on a class level dictionary so that is what was breaking test isolation | 14:10 |
| gary_poster | ah! cool. So you have to...stash the cache and put it back? | 14:11 |
| bac | so test that generate wadl need to clear the cache at the end | 14:11 |
| gary_poster | ah ok | 14:11 |
| bac | you think, or just clear? | 14:11 |
| gary_poster | generating the cache is expensive, I think | 14:11 |
| gary_poster | could be wrong | 14:11 |
| gary_poster | maybe time the two approaches | 14:11 |
| bac | no, it is very expensive | 14:11 |
| gary_poster | clearing would be nicer | 14:11 |
| bac | but do we want to keep it between tests? | 14:12 |
| gary_poster | but if you can just put it back and save a couple of minutes in the test...how long does it take? | 14:12 |
| gary_poster | we already do keep it between tests | 14:12 |
| gary_poster | if we didn't we'd be paying the cost for any of our tests like this | 14:12 |
| gary_poster | s/any/all/ | 14:12 |
| bac | well the problem is recognizing the tests that will be harmed by using a cached wadl that doesn't represent their view of the world | 14:13 |
| gary_poster | sure. | 14:14 |
| gary_poster | but...in either case | 14:14 |
| gary_poster | a well behaved test will need to either explicitly clear the cache | 14:14 |
| gary_poster | or reset the previous contents | 14:15 |
| gary_poster | if we put in automation to try to warn test writers (and I'm not quite sure how we would do that unless we stuff it in a shared base testcase), then why can't we put automation to always stash the contents on setup and reinstate on teardown? | 14:16 |
| bac | but if every test sets aside the cache in a setup and restores it in a teardown then none of those tests are using the cache and gaining no benefit from it | 14:22 |
| bac | i'm actually pretty curious now as to why using the cached wadl breaks this test. the wadl generation looks pretty innocuous | 14:26 |
| gary_poster | bac, no, that's not my idea | 14:36 |
| gary_poster | idea: in setUp, you make a copy of the cache | 14:36 |
| gary_poster | in tearDown, you reinstate that copy | 14:36 |
| bac | right | 14:36 |
| gary_poster | every test gets a new, clean copy of the cache | 14:36 |
| gary_poster | already populated | 14:37 |
| bac | but who generates the cache? | 14:37 |
| gary_poster | yeah, was thinking about that :-) | 14:37 |
| bac | if testA generates the cache, but in teardown replaces it with what he found at the beginning, then it is reset to {} | 14:37 |
| bac | i'm currently looking at the difference in the wadl between the cached and newly generated to see if i can tell why it is causing the failure | 14:38 |
| gary_poster | setUp could look on a local base class testcase attribute for the cache. If it does not exist, it clears out the wadl cache and forces it to be generated (I think there's an explicit call for that IIRC) | 14:40 |
| gary_poster | it then gets the generated code. | 14:40 |
| gary_poster | cache I mean | 14:40 |
| gary_poster | OTOH if the local version exists, it puts a copy in the cache. | 14:40 |
| gary_poster | eh, I dunno. | 14:40 |
| gary_poster | difference: doesn't it explicitly specify a different file to load in that test? | 14:41 |
| gary_poster | been a while since I looked at it | 14:41 |
| frankban | bac, gary_poster: I know nothing about how wadl is generated, but, could it be the wadl version? we have devel, beta and 1.0 | 14:42 |
| bac | frankban: all three versions are cached and used based on version | 14:42 |
| frankban | bac: hum... | 14:42 |
| gary_poster | yeah | 14:43 |
| bac | gary_poster, frankban: so the difference between the old and new wadl is simply https vs http | 14:52 |
| gary_poster | huh | 14:52 |
| bac | so the cached version has https URL, which cause the api tests to fail with 'connection refused' b/c they are listening for http connections | 14:53 |
| gary_poster | and in general--outside of tests--they are supposed to have https urls if I read the code correctly | 14:55 |
| frankban | test_wadl_generation seems to be the only test that uses generate_wadl, the latter calls _generate_web_service_root that do: | 14:58 |
| frankban | does: | 14:58 |
| frankban | # Since we want HTTPS URLs we have to munge the request URL. | 14:58 |
| frankban | url = url.replace('http://', 'https://') | 14:58 |
| frankban | gary_poster, bac: we could just add a secure=True argument to generate_wadl and _generate_web_service_root and then call generate_wadl(version, secure=False) in test_wadl... | 15:02 |
| gary_poster | I s'pose, but in general we seem to have code that is generating a test-friendly wadl cache somewhere | 15:03 |
| gary_poster | that code might change | 15:03 |
| gary_poster | if it does, we'd want that change too | 15:04 |
| gary_poster | we could try to figure out that mechanism | 15:04 |
| gary_poster | and comment it or hook into it or something | 15:04 |
| gary_poster | so as to make that approach more reliable | 15:04 |
| gary_poster | for the future | 15:04 |
| gary_poster | or so as to figure out another similar solution | 15:04 |
| gary_poster | utilities/create-lp-wadl-and-apidoc.py ... | 15:05 |
| gary_poster | which has no https->http code I see... | 15:07 |
| gary_poster | and looking at the wadl files in my filesystem, they are https too... | 15:09 |
| gary_poster | so who is fixing up https -> http? | 15:09 |
| gary_poster | for the tests? | 15:09 |
| bac | i think _generate_web_service_root as frankban pointed out above | 15:10 |
| gary_poster | bac, no, the other way around | 15:12 |
| gary_poster | bac, you said that our testing cache has http, right | 15:12 |
| gary_poster | and after we make this call, the urls are https | 15:12 |
| bac | yes, i was backwards | 15:13 |
| gary_poster | which time? :-) | 15:13 |
| gary_poster | so, it starts out https | 15:13 |
| gary_poster | and after that test it is http? | 15:13 |
| bac | test_wadl via generate_wadl generated https | 15:14 |
| frankban | gary_poster: I think after test_wadl it remains https, and it is cached | 15:14 |
| bac | i don't know who makes the http version used by the api tests | 15:14 |
| gary_poster | right | 15:14 |
| gary_poster | that was my question | 15:15 |
| frankban | gary_poster, bac: there is the possibility that the cached http version is created by the first test that exercises the api | 15:15 |
| gary_poster | I believe that is the case--oh! | 15:16 |
| bac | yes | 15:17 |
| gary_poster | right, I forgot about that. So, that means that initially there is no reading of the wadl files | 15:17 |
| gary_poster | then we produce the wadl files, using generate_wadl, which is done in such a way as to enforce https | 15:17 |
| frankban | my suggestion was like: if we can establish that test_wadl is the only test that uses https, we can switch that test to use http instead, so that the cached wadl is valid for all tests | 15:17 |
| gary_poster | right, I followed that. My concern was that other code may want other changes for tests. But, generate_wadl appears to be the only official entry point for this. | 15:18 |
| bac | not wadl files, but in-memory cached wadl | 15:19 |
| gary_poster | yes, good correction, thanks | 15:19 |
| bac | launchpadlib_for defines the service_root to be 'http://api.launchpad.dev/' | 15:21 |
| bac | so any test using that part of testing/webservice will get http URLs into the cache | 15:21 |
| bac | unless it is already populated | 15:21 |
| gary_poster | ok, I feel like you two have been standing there with the full undestanding all along. :-) Mm, so bac's suggestion was clearing out the cache after the test, and frankban's suggestion is to make the test produce http. Um, I'm fine with you all deciding. I lean more towards the "get rid of the stuff we generated in this test" approach rather than the "generate stuff in this test that we an use later," but I'm h | 15:22 |
| gary_poster | appy to let you all decide. | 15:22 |
| bac | so if our normal mechanism properly generates http and only test_wadl is generating https, then i think we should either fix it to generate http or clear the cache when it is done | 15:22 |
| bac | frankban's solution is good in that it preserves the cache | 15:23 |
| bac | frankban's solution is bad in that caching is bad for test isolation | 15:23 |
| frankban | I agree | 15:25 |
| frankban | another similar solution (with the same pros and cons) is to change _generate_web_service_root to switch http/https only if the version is not "devel", assuming that other tests use 'devel' | 15:28 |
| bac | i'm happy in that we now *fully* understand this hateful problem, so any solution is good. | 15:30 |
| bac | here is what i propose: http://pastebin.ubuntu.com/981864/ | 15:31 |
| bac | if test_wadl is the first wadl generator then there will be repeated effort but in all other situations there is no waste | 15:32 |
| frankban | bac: I like that only the test code is changed (using your solution), +1 | 15:39 |
| bac | yeah, that appealed to me too | 15:40 |
| bac | gary_poster: there are no other reviewers (but me), so if you have time, https://code.launchpad.net/~bac/launchpad/bug-996773-2/+merge/105499 | 15:58 |
| * bac ->lunch | 15:58 | |
| gary_poster | sure bac | 15:59 |
| gary_poster | frankban, call now or next week? | 15:59 |
| frankban | gary_poster: now is ok if you are have time | 16:00 |
| gary_poster | cool frankban. let's go to https://talkgadget.google.com/hangouts/_/extras/canonical.com/goldenhordeoneonone | 16:01 |
| gary_poster | sorry bac | 17:33 |
| gary_poster | was on call and then had lunch | 17:33 |
| gary_poster | but sinzui approved, as I'm sure you saw | 17:33 |
| gary_poster | bac, hey | 19:34 |
| gary_poster | you around for a quick call? | 19:35 |
| bac | oh yeah | 19:35 |
| bac | good thing you fixed your calendar. :) | 19:35 |
| bac | gary_poster: in oneonone | 19:36 |
| gary_poster | bac, I got a test error on the data center machine which looks like the old bzr whoami problem. Do we have to do something in the system to prevent these? | 20:23 |
| gary_poster | v | 20:23 |
| gary_poster | http://pastebin.ubuntu.com/982322/ | 20:23 |
| gary_poster | Ah, yeah I think I remember | 20:24 |
| bac | no, we rolled back the /etc/mailname approach | 20:24 |
| bac | and added a 'bzr whoami' to the failing test | 20:25 |
| bac | so the test should not fail | 20:25 |
| gary_poster | oh | 20:26 |
| gary_poster | maybe is this a different failing test, bac? | 20:26 |
| gary_poster | and we just need to add the same fix? | 20:26 |
| bac | gary_poster: this just posted http://www.youtube.com/watch?v=WAlhLz3lNNs&feature=uploademail "Juju Charm Best Practices" | 20:26 |
| bac | i wonder if any of ours made the cut | 20:26 |
| gary_poster | well, python shelltoolbox must not have :-( | 20:28 |
| bac | gary_poster: that test does not match what i see in trunk | 20:29 |
| bac | e.g. line 187, where the failure occurs is not the same | 20:29 |
| bac | do you have a recent branch? | 20:29 |
| bac | in trunk, the whoami fix is at line 179 | 20:30 |
| gary_poster | oh this is r15131 | 20:30 |
| bac | < 15236 | 20:30 |
| gary_poster | ah! | 20:31 |
| gary_poster | ok | 20:31 |
| gary_poster | thanks bac. I thought something was already updating the branch | 20:37 |
| bac | i have the UDS session playing in the background on my tv | 20:37 |
| bac | i keep hearing "best practices" and "promulgate" being used way too much | 20:37 |
| gary_poster | heh | 20:38 |
| gary_poster | bac, I have it in the background too. AFAICT they are not sharing best practices but just randomly saying blue sky stuff. oops | 20:38 |
| bac | yeah, well | 20:40 |
| bac | you know, if we're supposed to plugging in more company-wide, we really need to be at UDS | 20:40 |
| bac | gary_poster: so far i cannot reproduce bug 998040 | 20:44 |
| _mup_ | Bug #998040: lp.codehosting.tests.test_bzrutils.TestGetBranchStackedOnURL.testGetBranchStackedOnUrlNotStacked(RemoteBranchFormat-v2) fails rarely/intermittently in parallel tests <paralleltest> <Launchpad itself:In Progress by bac> < https://launchpad.net/bugs/998040 > | 20:44 |
| gary_poster | bac, agree about UDS | 20:45 |
| bac | running original worker set repeatedly now | 20:45 |
| gary_poster | no idea about repro :-/ | 20:45 |
| bac | ok | 20:45 |
| gary_poster | I just turned off the UDS session. kind of annoying. :-P | 20:46 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!