| lifeless | lib/lp/app/templates/root-index.pt is hilarious | 00:17 |
|---|---|---|
| lifeless | <div id="homepage-featured" class="homepage-portlet" | 00:17 |
| lifeless | tal:content="cache:anonymous, 1 hour"> | 00:17 |
| lifeless | <tal:cache | 00:18 |
| lifeless | tal:content="cache:public, 5 minutes" tal:omit-tag=""> | 00:18 |
| lifeless | yes, thats two conflicting cache rules in the same file, adjacent. | 00:18 |
| mwhudson | is the latter nested in the former? | 00:18 |
| lifeless | why yes, funny you should ask that. | 00:18 |
| mwhudson | awesome | 00:18 |
| lifeless | line 215,216,217,218 there | 00:18 |
| * james_w waves to NZ | 00:22 | |
| lifeless | I thought we had something that injected values into memcache for the home page | 00:24 |
| lifeless | but I cannae find it | 00:24 |
| lifeless | hi james_w | 00:24 |
| mwhudson | hi james_w | 00:25 |
| mwhudson | lifeless: that sounds like something that would be suspiciously well designed for lp memcache | 00:25 |
| lifeless | bitter bitter | 00:26 |
| * james_w now has tarmac updating sourcedeps.conf for config-manager, so that staging can autorollout changes across any branch involved in a deployment | 00:27 | |
| james_w | the next target should be the latency before deployment-manager sees that | 00:28 |
| lifeless | nice | 00:28 |
| lifeless | I would like to drop update-sourcedeps and put LP back to config-manager | 00:28 |
| james_w | yeah | 00:28 |
| lifeless | since the functionality is basically identical, except that the sysadmins use config-manager :) | 00:28 |
| james_w | I was pondering about replacing config-manager with bzr-builder the other day | 00:28 |
| james_w | but I'm not sure there's any benefit | 00:29 |
| lifeless | 6/1 1/2 of the other | 00:29 |
| james_w | except for having one fewer codebase | 00:29 |
| lifeless | well, neither is a superset of the other | 00:29 |
| lifeless | so I don't think you'd achieve that in the first instance. | 00:29 |
| james_w | the reason I was thinking of that is that I thought it would be useful to have the "only do it if something is changed" feature of bzr-builder | 00:29 |
| lifeless | ah, so there is something like that in update-sourcedeps | 00:30 |
| james_w | but talking with web-ops led me to believe that having tarmac update things rather than relying on that is a better way to go | 00:30 |
| lifeless | should be straight forward enough to give config-manager a similar hinting facility | 00:30 |
| wgrant | lifeless: Those memcache rules you quoted aren't necessarily wrong. They could be quite reasonable. | 00:30 |
| lifeless | wgrant: orly | 00:30 |
| james_w | the only other small benefit I could see is the "manifest" that bzr-builder writes | 00:31 |
| wgrant | lifeless: And no, nothing injects. That was a proposal to remove HTTP access from the appservers. | 00:31 |
| lifeless | wgrant: you mean if the inner one ends short | 00:31 |
| james_w | that is valid input, so you could deploy exactly to production what was in staging by linking up the output of one to the input of the other | 00:31 |
| james_w | but we are close enough to that already I think | 00:31 |
| lifeless | wgrant: and even if it ends short, its wrong | 00:32 |
| lifeless | wgrant: you have to have your *longest* cache times at the innermost context. | 00:32 |
| wgrant | lifeless: It may be *misguided*. | 00:32 |
| wgrant | lifeless: But it's perfectly valid. | 00:32 |
| wgrant | lifeless: Why? | 00:32 |
| wgrant | The top-level one caches for an hour for anonymous users. | 00:32 |
| wgrant | Non-anonymous users get lower expiration times. | 00:32 |
| lifeless | anyhow, gones now. | 00:33 |
| lifeless | there are precisely two uses of memcache I'm a little wary of removing, but I'm willing to take the risk. | 00:33 |
| lifeless | I may need to go back and feature flag this change, somehow. | 00:33 |
| lifeless | will see | 00:33 |
| wgrant | The frontpage blogpost one can't really be removed. | 00:34 |
| wgrant | The others can. | 00:34 |
| lifeless | that is one of them | 00:35 |
| lifeless | and it can be | 00:35 |
| wgrant | lol | 00:35 |
| wgrant | Let's not call into a frequently down Wordpress on every RootObject:+index load,l please. | 00:35 |
| lifeless | the other one is the global counts on RootObject:index.html | 00:38 |
| lifeless | aieee pageidblindness RootObject:+help-blueprints for /srv/launchpad.net/production/launchpad-rev-15190/lib/lp/blueprints/help | 00:39 |
| wgrant | lifeless: SELECT MAX(id) :) | 00:39 |
| lifeless | wgrant: they are currently cached, I'm assuming there is some insanity lurking. | 00:39 |
| wgrant | There is. | 00:40 |
| lifeless | wgrant: so, we're meant to be using squid in the DC everywhere | 00:40 |
| wgrant | Which is why you should SELECT MAX(id) | 00:40 |
| wgrant | lifeless: Ha ha ha | 00:40 |
| wgrant | lifeless: We do. | 00:40 |
| lifeless | wgrant: we get sensible headers from this wordpress: | 00:40 |
| lifeless | Last-Modified: Wed, 06 Jun 2012 15:15:54 GMT | 00:40 |
| lifeless | ETag: "3fafd55d87f99a8d01f330c4402a3b75" | 00:40 |
| wgrant | lifeless: Whether it's not broken or actually useful is another quesiton. | 00:40 |
| wgrant | Oh | 00:40 |
| wgrant | That's surprising. | 00:40 |
| lifeless | wgrant: so other than a check with webops that appservers are actually using it | 00:40 |
| lifeless | I'm not concerned. | 00:40 |
| lifeless | it is 37K of html to parse. | 00:41 |
| wgrant | lifeless: We also need to fix it to not crash if it can't retrieve it. | 00:41 |
| lifeless | but we can move the caching into the model rather than template if we want to | 00:41 |
| wgrant | lifeless: We've used this as fault-tolerance in the past, and everything goes to shit if the cache happens to expire while it's down. | 00:42 |
| lifeless | wgrant: since you are here. | 00:43 |
| lifeless | wgrant: I'd like your opinion on the tal:cache in lib/lp/app/templates/base-layout-macros.pt | 00:43 |
| lifeless | AFAICT the entire block is removable as something insane | 00:43 |
| lifeless | but, I'm likely misunderstanding it | 00:44 |
| wgrant | You are completely misunderstanding it | 00:44 |
| wgrant | It's unrelated to memcache; run away | 00:44 |
| lifeless | <tal:cache condition="view/user|nothing" | 00:44 |
| lifeless | ^ that memcaches its output | 00:45 |
| wgrant | Nope | 00:45 |
| wgrant | tal:FOO | 00:45 |
| wgrant | The FOO means nothing. | 00:45 |
| wgrant | Only the NS is important. | 00:45 |
| lifeless | Oh, freaking lies. | 00:45 |
| lifeless | lies lies lies. | 00:45 |
| lifeless | Have I mentioned I loath some parts of tal ? | 00:45 |
| wgrant | It just implies a no-op element and removes the need for the NS on attribute names. | 00:45 |
| lifeless | grep caught it | 00:46 |
| lifeless | cause tal:cache was used by folk to setup memcache rules. | 00:46 |
| lifeless | thanks for 'splaining. | 00:46 |
| lifeless | be nice to give it a clearer name. | 00:46 |
| wgrant | np | 00:46 |
| lifeless | my lp:memcache branch will be massive LoC win soon :) | 00:47 |
| lifeless | well, s/massive/enoughtodosomethinginterestingwith/ | 00:48 |
| lifeless | and pushing \o/ | 00:49 |
| lifeless | mwhudson: bug 623199 lies | 00:52 |
| _mup_ | Bug #623199: scripts do not establish valid zope participations <lp-foundations> <qa-untestable> <Launchpad itself:In Progress by mwhudson> < https://launchpad.net/bugs/623199 > | 00:52 |
| mwhudson | the 'in progress' part? | 00:52 |
| lifeless | and the assignee I suspect, part and parcel | 00:53 |
| lifeless | unless you are ... | 00:53 |
| mwhudson | yes certainly that | 00:53 |
| mwhudson | i guess it's a classic of not really specifying a condition of when it can be called done | 00:53 |
| mwhudson | as i pointed out early on the title is also a lie, after all | 00:53 |
| lifeless | wgrant: bug 939055 - its alredy mmmmmmmmmm horrible | 00:53 |
| _mup_ | Bug #939055: front page blog feed updates are done in-line with requests <canonical-losa-lp> <Launchpad itself:Triaged> < https://launchpad.net/bugs/939055 > | 00:53 |
| lifeless | sorry, cat got in there | 00:54 |
| wgrant | lifeless: Horrible but working. | 00:54 |
| lifeless | wgrant: yes, and when the blog is down, and memcache times out, the behaviour we see will be the same | 00:54 |
| wgrant | No. | 00:54 |
| mwhudson | lifeless: if you want to retitle it as "timelines are not automatically cleared when a new interaction is set up" then it's back to triaged | 00:54 |
| lifeless | wgrant: you'll need to unpack that. | 00:54 |
| wgrant | lifeless: We'll see it when the blog is down. | 00:55 |
| wgrant | No memcache timed out required. | 00:55 |
| wgrant | Currently a memcache timeout is required. | 00:55 |
| lifeless | wgrant: which is a mild increase in frequency increase | 00:55 |
| wgrant | lifeless: Before it can be removed we need to prevent it from crashing. | 00:55 |
| lifeless | wgrant: a 10 minute outage today has a 1 in 6 chance of showing the same crash. | 00:55 |
| mwhudson | lifeless: if you want to retitle it "scripts use interactions which are not suitable for storing interesting things on" then its fix released | 00:55 |
| lifeless | wgrant: so no, we don't 'need' to. | 00:55 |
| mwhudson | (and you should file/find a new bug for that first thing) | 00:56 |
| wgrant | lifeless: This means it's impossible to perform any maintenance on the blog without bringing parts of Launchpad down. | 00:56 |
| lifeless | mwhudson: FR it please. | 00:56 |
| lifeless | wgrant: no, it doesn't, because it will still be cached, and we can trivially make that page a 60 minute cache as well. | 00:56 |
| lifeless | wgrant: via a one-line config change to squid. | 00:56 |
| wgrant | Ah good, SPOFing ourselves even more on druzhnaya :) | 00:57 |
| mwhudson | lifeless: https://bugs.launchpad.net/launchpad/+bug/623199 | 00:57 |
| _mup_ | Bug #623199: there is no way to store interesting data on the participation scripts set up <lp-foundations> <qa-untestable> <Launchpad itself:Fix Released by mwhudson> < https://launchpad.net/bugs/623199 > | 00:57 |
| lifeless | wgrant: vs memcaches non-resilient cluster, no significant difference. | 00:57 |
| lifeless | mwhudson: thanks | 00:57 |
| wgrant | lifeless: Well, the memcached cluster is not an Antarctic base and we haven't had major problems with it in the past. | 00:58 |
| lifeless | wgrant: so put numbers on this; whats the expect change, in % of our OOPS count per month. | 00:59 |
| lifeless | wgrant: did you remove the webservice entity cache stuf? | 01:04 |
| wgrant | lifeless: It's gone from LP, but not from lazr.restful. | 01:04 |
| lifeless | kk | 01:05 |
| lifeless | darrens has confirmed, appservers are using druz for it. | 01:06 |
| lifeless | and we're getting -lots- of hits on it | 01:08 |
| lifeless | wgrant: another thing you've missed; you're assuming the front page is staying in memcache | 01:08 |
| lifeless | last I checked our stats, we have a ridiculously low hit rate, partly due to high evictions. | 01:09 |
| lifeless | allenap: are you really hacking on bug 704585 ? | 01:09 |
| _mup_ | Bug #704585: canonical_url performs poorly <timeout> <Launchpad itself:In Progress by allenap> < https://launchpad.net/bugs/704585 > | 01:09 |
| lifeless | wgrant: bug 618019 | 01:14 |
| _mup_ | Bug #618019: storm processing of result sets can be very very slow <lp-foundations> <Launchpad itself:Triaged> <Storm:Confirmed> < https://launchpad.net/bugs/618019 > | 01:14 |
| lifeless | wgrant: just retitled. Thats the one about cache handling and columns etc. | 01:14 |
| lifeless | wgrant: and/or bug 632145 | 01:15 |
| _mup_ | Bug #632145: handling of Specification result sets with 3000 rows is slow <Storm:New> < https://launchpad.net/bugs/632145 > | 01:15 |
| lifeless | hmm, can't find the bug about series/milestones having caching | 01:17 |
| james_w | anyone know if there's a standard for publishing structured content in an atom feed? | 01:25 |
| lifeless | hmm, we may have fun :) - 24 / 0 RootObject:index.html | 01:25 |
| james_w | i.e. for PuSH | 01:25 |
| lifeless | james_w: 'atom' | 01:26 |
| james_w | for something that isn't author/description/etc. | 01:26 |
| james_w | description = json.dumps(whatever) perhaps | 01:26 |
| lifeless | james_w: its xml | 01:27 |
| lifeless | james_w: putting json in xml would be fffffugly at best :) | 01:27 |
| lifeless | james_w: however, atom has no DTD or validator | 01:28 |
| lifeless | james_w: so, IIRC, you can just add any keys you want | 01:28 |
| lifeless | http://tools.ietf.org/html/rfc4287#section-6 | 01:29 |
| lifeless | james_w: ^ | 01:30 |
| lifeless | james_w: are you looking at PSHB ? | 01:30 |
| lifeless | mwhudson: https://code.launchpad.net/~lifeless/launchpad/memcache/+merge/109551 if yo ufeel like a treat. | 01:35 |
| james_w | lifeless, yes | 01:36 |
| lifeless | james_w: I'm very keen on PSHB, FWIW. | 01:37 |
| james_w | lifeless, the next question is how to convince a library to add arbitrary content :-) | 01:37 |
| james_w | lifeless, I know, the two are related :-) | 01:37 |
| lifeless | james_w: :) | 01:37 |
| === lifeless changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2 | ||
| lifeless | james_w: did your oops-tools patch fail ? | 03:39 |
| lifeless | james_w: or we just haven't deployed. | 03:39 |
| james_w | lifeless, the branding one? | 03:39 |
| lifeless | yes | 03:40 |
| james_w | lifeless, it's waiting on tarmac or something | 03:40 |
| lifeless | james_w: webops run that, and don't want the MP's, so a ping on -ops is the thing (which I've just done) | 03:47 |
| james_w | ok | 03:48 |
| lifeless | can has revu https://code.launchpad.net/~lifeless/launchpad/bug-1011390/+merge/109559 | 03:54 |
| stub | diff updating seems stuck there | 04:06 |
| lifeless | I see a diff | 04:08 |
| stub | r=stub | 04:10 |
| stub | I saw a diff. I also saw the yellow diff-is-updating blurb. Odd since there is only one unmerged revision? | 04:10 |
| lifeless | huh, yes | 04:12 |
| lifeless | that is indeed odd | 04:12 |
| lifeless | I only pushed to it the once | 04:12 |
| lifeless | stub: this may be more interesting https://code.launchpad.net/~lifeless/launchpad/memcache/+merge/109551 | 04:12 |
| lifeless | stub: could you ec2-land the former branch? Still on my TODO is fixing my landing environment. | 04:12 |
| stub | haha... when I move upstairs, yes :) | 04:13 |
| lifeless | stub: no rush | 04:13 |
| lifeless | stub: have a good weekend ? | 04:13 |
| stub | good enough ;) | 04:13 |
| lifeless | brb | 04:14 |
| stub | Did sod all besides a movie. | 04:14 |
| lifeless | nice | 04:16 |
| lifeless | I'm finally, as of late sunday, feeling human again | 04:16 |
| lifeless | small bit of sniffles is all that remains | 04:16 |
| lifeless | of course, cynthia is now teething again and didn't sleep at all last night >< | 04:16 |
| * lifeless is spaced out | 04:16 | |
| stub | huh... I also see just 'updating diff' on that memcache mp | 04:16 |
| adeuring | good morning | 07:30 |
| allenap | lifeless: No, I'm not. I've set it back to triaged. | 08:17 |
| === almaisan-away is now known as al-maisan | ||
| jml | Review of https://code.launchpad.net/~jml/launchpad/validate-ppa-owner/+merge/109529 sought. | 08:25 |
| jml | also, how can I tell when my db patch has been deployed? | 08:27 |
| cjwatson | Watch http://lpqateam.canonical.com/qa-reports/deployment-db-stable.html; also IME one of the .au cabal will often tell you | 08:28 |
| cjwatson | You can also watch https://wiki.canonical.com/InformationInfrastructure/OSA/LaunchpadProductionStatus | 08:28 |
| jml | cjwatson: thanks. | 08:29 |
| jml | cjwatson: how did you qa your db patch? | 08:31 |
| Gwaihir | hello all, I have a small problema with a django app that is using launchpad lib to get user info, it gives an HTTPError code 401, saying the the token has expired, does anyone have an idea on how to this could be fixed? bug is #1010455 (repeating question from #launchpad channel) | 08:33 |
| _mup_ | Bug #1010455: Patchmetric Login Token Expired <Linaro patch metrics:New> < https://launchpad.net/bugs/1010455 > | 08:33 |
| cjwatson | jml: ran test suite on corresponding code branch with db patch both unapplied and applied; asked on -ops for somebody to tell me how long the db patch took to apply on (qa)staging so that I could ensure it was within the time limit | 08:35 |
| jml | cjwatson: ok. thanks. | 08:36 |
| cjwatson | (which looks like /srv/launchpad.net-logs/staging/sourcherry/*-staging_restore.log on carob) | 08:37 |
| === jamestunnicliff_ is now known as jamestunnicliffe | ||
| jml | LP's permission system :( | 10:05 |
| jml | I can figure out how to allow a new group of people the ability to *create* private PPAs, but I can't figure out how to let them *set* private. | 10:21 |
| jml | Hmm. Hmm. | 10:23 |
| === al-maisan is now known as almaisan-away | ||
| czajkowski | jml: do you need any branches now made private if you do just shout and I can do it for you. | 10:25 |
| jml | czajkowski: I think I'm OK thanks. | 10:26 |
| === almaisan-away is now known as al-maisan | ||
| === matsubara is now known as matsubara-lunch | ||
| rick_h | morning | 10:38 |
| rick_h | ivorykr8: howdy, welcome to the squad | 10:39 |
| === benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: benji | Firefighting: - | Critical bugs: 3.47*10^2 | ||
| gary_poster | ivorykr8, welcome. don't believe allenap, we're bots. | 12:09 |
| rick_h | gary_poster: that's just what a non-bot would say! | 12:12 |
| gary_poster | rick_h, my brother always lies. I always tell the truth. | 12:12 |
| === matsubara-lunch is now known as matsubara | ||
| jml | oh hey | 12:23 |
| jml | on call reviewers! | 12:23 |
| rick_h | anyone recall if there's a way to python -m module:function ? | 12:24 |
| rick_h | I can get the module part, but not the function in the module | 12:24 |
| jml | benji: https://code.launchpad.net/~jml/launchpad/validate-ppa-owner/+merge/109529 is the first of three patches that needs review. | 12:24 |
| jml | rick_h: python -c 'import module; module.function()' | 12:24 |
| benji | jml: I'll take a look. | 12:25 |
| jml | benji: thanks. | 12:26 |
| rick_h | jml: thanks | 12:41 |
| === jam1 is now known as jam | ||
| jml | benji: sinzui has just approved that branch. https://code.launchpad.net/~jml/launchpad/p3a-private-team/+merge/109600 is the next one in the series. | 13:08 |
| benji | jml: ok | 13:09 |
| === fjlacoste changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | Welcome our new intern: ivorykr8 | On call reviewer: benji | Firefighting: - | Critical bugs: 3.47*10^2 | ||
| fjlacoste | ivorykr8: welcome! | 13:38 |
| jelmer | 'morning fjlacoste | 13:39 |
| jelmer | hi ivorykr8 | 13:39 |
| === fjlacoste is now known as flacoste | ||
| flacoste | hi jelmer | 13:40 |
| rvba | Hi ivorykr8 and welcome to the team! | 13:41 |
| deryck | ivorykr8, welcome! Here's the link for the stand-up: https://plus.google.com/hangouts/_/4b599a50a7ebb329230ced7d9caef1ba95835bfc?authuser=0&hl=en | 14:02 |
| deryck | adeuring, likewise ^^ | 14:02 |
| === al-maisan is now known as almaisan-away | ||
| mterry | Hello! If I can see a private bug, I had thought that meant I can edit its privacy. But I've encountered a crasher where that's not the case. Is there a way to find out why I can't edit the privacy? | 14:50 |
| mterry | (this is bug 1011293) | 14:52 |
| mterry | No bug bot? :( https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/1011293 | 14:53 |
| bac | welcome ivorykr8 | 15:05 |
| czajkowski | ivorykr8: how are you doing on day 1 anything we can help you with ? | 15:18 |
| ivorykr8 | well actually no i just try to get myself into python :) | 15:21 |
| jml | https://code.launchpad.net/~jml/launchpad/remove-top-tests/+merge/109664 up for review | 15:32 |
| jml | (Heals 477 lines of damage) | 15:32 |
| cjwatson | benji: Thanks for your review on https://code.launchpad.net/~cjwatson/launchpad/pocket-permissions/+merge/109192; can you "Status: Approved" and I'll land it? | 15:35 |
| benji | cjwatson: sure | 15:35 |
| james_w | jml, delete it in the other branch too and score double! | 15:36 |
| jml | james_w: well, ISTR objecting when it was first added. | 15:37 |
| james_w | I assume that was before testr slowest was added? | 15:45 |
| jml | james_w: yeah, but it was after subunit reports were available from bin/test, I think. | 16:37 |
| lifeless | morning everyone | 17:50 |
| jml | is there a LoC score board? | 18:01 |
| lifeless | other than ohlo, no. | 18:01 |
| lifeless | and cjwatsons thing | 18:02 |
| jml | ta | 18:03 |
| * jml has been mucking around with dmg | 18:04 | |
| lifeless | benji: hi | 18:05 |
| benji | hi lifeless | 18:05 |
| lifeless | benji: if you're still on the review hook, perhaps you can do me a related favour | 18:05 |
| lifeless | 05:59 < lifeless> anyone around to run an ec2 test command for me? Will get my environment fixed soon, I swear. | 18:05 |
| lifeless | 05:59 < lifeless> https://code.launchpad.net/~lifeless/launchpad/memcache/+merge/109551 | 18:05 |
| lifeless | (I asked in the wrong channel initially :P) | 18:05 |
| benji | lifeless: Instance starting. I'll give you the URL when it presents itself. | 18:07 |
| lifeless | benji: oh doh | 18:07 |
| lifeless | benji: I mean 'ec2 land' | 18:07 |
| lifeless | not ec2 test. | 18:07 |
| benji | heh | 18:07 |
| benji | ok, restarting | 18:07 |
| * lifeless stabs his lying fingers. | 18:07 | |
| benji | heh | 18:08 |
| lifeless | benji: thanks | 18:08 |
| jml | g'night | 18:10 |
| === jcsackett_ is now known as jcsackett | ||
| lifeless | night jml | 18:10 |
| === matsubara is now known as matsubara-afk | ||
| lifeless | benji: could you try again, if you're still around ? | 21:39 |
| lifeless | benji: I've pushed a fix | 21:39 |
| jcsackett | wallyworld_, StevenK, wgrant: i don't think i'm making standup--i've been feeling progressively iller as the afternoon has worn on and i'm at the point where i just need to crash. | 21:50 |
| jcsackett | s/iller/more ill/ | 21:50 |
| === salgado is now known as salgado-afk | ||
| wallyworld_ | jcsackett: sorry to hear that, get well soon | 22:01 |
| benji | lifeless: I was AFK. Running ec2 land for you now. | 22:22 |
| lifeless | benji: thanks! | 22:32 |
| StevenK | wgrant: Brian's branch also needs QA. | 22:45 |
| StevenK | And lifeless' r15388 | 22:45 |
| wgrant | StevenK: Yes, but they're rather simple. | 22:46 |
| * StevenK looks for an open team on qas to abuse | 22:46 | |
| lifeless | StevenK: mine doesn't, just qa-ok it; no code changes | 22:53 |
| lifeless | config default value change + increased test coverage. | 22:53 |
| StevenK | wgrant: So my change sort of works. If you select/type an open team and hit Subscribe, the form submits and dumps you back to Branch:+index with no feedback, but they are not subscribed. | 22:56 |
| wgrant | StevenK: So the form has no error handling? | 23:00 |
| StevenK | wgrant: My code calls setFieldError(), so I'm not sure why that doesn't fire. | 23:01 |
| wgrant | StevenK: Simply setting a fielderror won't do much if the form isn't rendered. | 23:07 |
| wgrant | StevenK: You probably want to not set next_url in that case | 23:07 |
| wgrant | So it doesn't redirect away | 23:07 |
| wgrant | Normally the validation is done in a validate method, not the action itself. | 23:07 |
| StevenK | wgrant: So I think the code may need a little more polish -- use a validate method on Branch:+subscribe like you say, and probably change the JS popup if the branch is private and the reviewer is an open team when proposing an MP. | 23:09 |
| wgrant | validate may be less appropriate here. | 23:09 |
| wgrant | LBYL is perhaps not a good idea. | 23:10 |
| StevenK | wgrant: Oh? What do you suggest then? A notification that "so-and-so not subscribed, this branch is private and they are an open/delegated team." ? | 23:11 |
| wgrant | Personally I would alter the JS popup to not change anything, and just tell you that you're doing something stupid. Since it should become rare that people don't have access. | 23:11 |
| wgrant | StevenK: I'd probably have the action catch the exception and display an error. | 23:11 |
| wgrant | Like you do now, except in a way that actually works. | 23:11 |
| StevenK | Haha | 23:12 |
| StevenK | I thought setFieldError() would work :-( | 23:12 |
| wgrant | It sets a field error. | 23:12 |
| StevenK | Creating an MP with an open team as a reviwer has them pop up as the reviewer, but they don't get subscribed. | 23:14 |
| StevenK | wallyworld_, wgrant: Parramatta got 90mm of rain yesterday. | 23:17 |
| wallyworld_ | is that all? | 23:17 |
| StevenK | Hah | 23:19 |
| StevenK | wgrant: So I think this is qa-ok for now, but I will come back to it after the DB patch o' doom. | 23:51 |
| wgrant | StevenK: Sounds good. | 23:53 |
| wgrant | Seems like you have a tonne of missing test coverage, thoguh. | 23:53 |
| StevenK | A test for the field error is there, and it passed. :-( | 23:53 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!