[00:00] <wgrant> I just uploaded and downloaded an attachment named 'huh?'
[00:00] <sinzui> wgrant, https://bugs.launchpad.net/ubuntu/+source/gnome-control-center/+bug/983116/+attachment/3077581/+files/HUD-to-do-what%3F-de.png
[00:00] <_mup_> Bug #983116: [Component: Keyboard] HUD is <Alt> not <Alt L> <amd64> <apport-bug> <hud> <precise> <gnome-control-center (Ubuntu):New> < https://launchpad.net/bugs/983116 >
[00:00] <wgrant> Yeah, prod strips them
[00:00] <wgrant> Compare the exception on https://launchpad.net/huh%3F and https://launchpad.dev/huh%3F
[00:01] <wgrant> https://dogfood.launchpad.net/huh%3F shows the correct behaviour
[00:01] <wgrant> qastaging is bad
[00:01] <wgrant> So something on the prod/qas frontend configs is molesting our URLs; there's no LP bug here.
[00:01] <sinzui> yes, jcsackett confirmed that qa and prodct beahve the same
[00:02] <wgrant> So I would suggest replacing that MP with an RT
[00:02] <wgrant> Who knows what else the frontends are doing :/
[00:02] <sinzui> and watch it for 5 years?
[00:03] <wgrant> Nah, I'll coerce APAC webops to give me the configs next week and then give them a diff :)
[00:03] <wgrant> Diffs are a good way to accelerate RTs
[00:03] <wgrant> I am glad it is reproducible on qastaging; that makes things much easier.
[00:04] <wgrant> Since there's no squid or haproxy, AIUI
[00:05] <StevenK> steven@undermined:~% wget -Sq https://qastaging.launchpad.net 2>&1 | grep X-Cache
[00:05] <StevenK>   X-Cache: HIT from arsenic.canonical.com
[00:05] <wgrant> Oh
[00:05] <wgrant> So *that's* what arsenic does...
[00:05] <wgrant> s/Oh/Oh god why/
[00:06]  * wgrant files the RT
[00:06] <StevenK> arsenic for staging too
[00:07] <wgrant> We may need to do the same tweak we did to get the restricted librarian working
[00:08] <wgrant>     # nocanon per RT#42560, woe with LP's handling of eg %2B
[00:08] <wgrant>     ProxyPass / balancer://launchpad-librarian-cluster/ nocanon
[00:08] <_mup_> Bug #42560: /var is mounted over /var/run and /var/lock <sysvinit (Ubuntu):Invalid> < https://launchpad.net/bugs/42560 >
[00:08] <wgrant> That shouldn't be it, since decoding the %3F is not a legal part of canonicalization, but it's possible.
[00:09] <wgrant> Whatever it is, either apache or squid is in the wrong here, not LP
[00:12] <wgrant> It looks like it's probably our squid config, but I don't think I have a copy.
[01:05]  * StevenK glares at DF
[01:06] <StevenK> With no builder how am I supposed to QA? :-(
[01:25] <wgrant> StevenK: DB hacking or qa-meh
[01:25] <StevenK> Yeah, qa-meh might be the way
[01:50] <StevenK> wgrant: I wonder if bug 834293 is fixed, we did a bunch of work on branch privacy
[01:50] <_mup_> Bug #834293: Product:+code-index times out <timeout> <Launchpad itself:Triaged> < https://launchpad.net/bugs/834293 >
[02:07] <wgrant> StevenK: No
[02:07] <wgrant> We made it better, but not much better
[02:07]  * wgrant finds data
[02:08] <wgrant> https://devpad.canonical.com/~lpqateam/ppr/lpnet/latest-monthly-pageids.html
[02:08] <wgrant> 99% under 7.26s
[02:11]  * StevenK made the mistake of searching OOPS on neem with 'AssertionError'
[02:12] <StevenK> 1648 OOPSes later ...
[02:13] <StevenK> Almost all of them appear to be AssertionError: Bug #504291: Store left in a disconnected state.
[02:13] <wgrant> StevenK: Why?
[02:13] <_mup_> Bug #504291: DisconnectionErrors (already disconnected) happening again <fastdowntime> <lp-foundations> <oops> <Launchpad itself:Incomplete by stub> <Storm:Invalid> < https://launchpad.net/bugs/504291 >
[02:13] <wgrant> We know the precise circumstances around +edit-dependencies
[02:13] <wgrant> So we don't need to grep for it
[02:13] <StevenK> I wasn't
[02:13] <StevenK> That grep was for bug 808950
[02:13] <_mup_> Bug #808950: AssertionError: You exported name as an IChoice based on an SQLObjectVocabularyBase, you should use lazr.restful.fields.ReferenceChoice instead <api> <oops> <Launchpad itself:Triaged> < https://launchpad.net/bugs/808950 >
[02:14] <wgrant> Ah
[02:15] <StevenK> But doesn't seem to show up
[02:23] <wgrant> StevenK: Have you tried to reproduce it?
[02:25] <StevenK> I was looking for an OOPS as the first step.
[02:26] <wgrant> "AssertionError: You exported milestone_assignment as an IChoice based on an SQLObjectVocabularyBase, you should use lazr.restful.fields.ReferenceChoice instead." on Product:EntryResource:searchTasks
[02:26] <wgrant> is pretty clear
[02:26] <wgrant> And the fix is obvious
[02:28] <wgrant> (hint: grep the entire tree for the field name)
[02:31] <StevenK>         # The vocabulary should be HWBus; this is fixed in
[02:31] <StevenK>         # _schema_circular_imports to avoid circular imports.
[02:31] <StevenK> Didn't you kill that?
[02:31] <wgrant> _schema_circular_imports continues to live
[02:31] <wgrant> Its death will come only from lazr.restful's demise, or being split up into app-specific _schema_circular_importses.
[02:32] <StevenK> wgrant: Hmm, I don't get it. Since bugtasksearch has assignee as a Choice
[02:32] <wgrant> StevenK: Look at the other places that use it, and it should become clear
[02:33] <StevenK> Hah. Assignee is a Reference
[02:33] <StevenK> In bugtarget
[02:33] <wgrant> That's fine.
[02:33] <StevenK> But milestone_assignment isn't
[02:33] <wgrant> We only care about milestone_assignment, don't we?
[02:33] <StevenK> We shouldn't use copy_field, but use Reference?
[02:34] <wgrant> I'd look at how the other code that goes near that field uses it.
[02:35] <StevenK> wgrant: What other code? milestone_assignment turns up in two places.
[02:35] <wgrant> You're on the right track.
[02:36] <StevenK> That wasn't the answer I was expecting. :-)
[02:37] <StevenK> wgrant: Oh, it isn't used at all./
[02:38] <StevenK> search params never assigns it in __init__
[02:38] <wgrant> Bingo
[02:39] <StevenK> Should I drop it entirely then?
[02:39] <StevenK> It's obviously exported, but use of it will return 500s
[02:39] <wgrant> Right, it's probably in 1.0, but it's also likely that any attempt to use it crashes
[02:39] <wgrant> Check that, and if it always crashes then obliterate it
[02:39] <wgrant> If it doesn't always crash, think about it a big and then obliterate it anyway.
[02:40] <wgrant> bit
[02:41] <StevenK> Hah
[02:41] <StevenK> So it wants a milestone name ?
[02:41] <wgrant> I think it wants a milestone reference.
[02:41] <wgrant> Given that it's using the Milestone vocab
[02:42] <wgrant> But it's exported as a Choice, which means it wants an enum value
[02:42] <wgrant> Which is why it crashes
[02:42] <wgrant> Try giving it references and names and see if there's anything that doesn't OOPS
[02:44] <StevenK> OOPS-a8ab241f5057264be5073a00263b04fb and OOPS-2c4ba13f50161b4d3ae38ccb94059308
[02:49] <wgrant> Ah... ha...
[02:49] <wgrant> The archeology of this is interesting
[02:49] <wgrant> It actually dates back to batch edit functionality added in mid-2005.
[02:50] <wgrant> In r1860
[02:50] <wgrant> If milestone_assignment was set in search parameters, it looks like it set the milestone of the found bug tasks.
[02:51] <StevenK> HAHAHA
[02:51] <StevenK> It's only referenced in a doctest that has been disabled since 2005
[02:51] <wgrant> Ah, no, there was a separate batch assignment form on the search page, but it all used that same "search" schema
[02:51] <StevenK> # XXX Bjorn Tillenius 2005-08-01:
[02:51] <StevenK> # This test is disabled in wait for SQLObject being fixed to accept
[02:51] <StevenK> # both distinct=True and an orderBy argument.
[02:52] <wgrant> xx-bugattachments.txt is disabled?
[02:52] <StevenK> No, just the last test of it
[02:52] <wgrant> Ah
[02:52] <StevenK>   XXX print http(r"""
[02:52] <StevenK>   ... GET /bugs/fir...
[02:53] <StevenK> wgrant: What about name in that bug? Or is that long dead?
[02:54] <wgrant> StevenK: It may still exist, but the pageid is bad
[02:54] <wgrant> Perhaps search the apidoc for parameters named name
[02:54] <wgrant> 'cause the method mentioned in the bug doesn't have onje
[02:54] <wgrant> And I don't think it ever did
[02:55] <wgrant> Huh
[02:55] <wgrant> That code persisted until June 2006
[02:55] <wgrant> But I'm pretty sure the bulk edit form was long gone by then, or I'd remember.
[02:57]  * StevenK ignores name
[04:02] <StevenK> wallyworld__: Read https://code.launchpad.net/~stevenk/launchpad/obliterate-milestone_assignment/+merge/128158 and weep
[04:06]  * wallyworld__ reads
[04:08] <wallyworld__> there were some cobwebs there
[04:08] <StevenK> I think the cobwebs are covered in dust
[04:09] <StevenK> Maybe this branch will land now
[04:13] <StevenK> wallyworld__: Did you want to approve that MP, or are you afraid of the spiders that own said cobwebs?
[04:15] <wallyworld__> StevenK: i'll aprove it, i just thought you wanted me to read it
[04:16] <StevenK> wallyworld__: The reading was first, yeah
[04:16] <StevenK> wallyworld__: I was curious about your reaction before you reviewed it
[04:16] <wallyworld__> StevenK: so, you saying that if the api is used now, as it, it oopses?
[04:16] <wallyworld__> as is
[04:17] <StevenK> wallyworld__: searchTasks() works fine. If you pass in milestone_assignment as one of your arguments, you will get an OOPS.
[04:17] <wallyworld__> ok, so no harm in removing it then
[04:17] <StevenK> Right, and I doubt any one is using it
[04:17] <StevenK> If they are, they move from a 500 to what, 404 or a lazr.restful error?
[04:18] <wallyworld__> you may have won the prize for removing the oldest code
[04:18] <wallyworld__> 40x for sure, not sure what
[04:18] <wgrant> 400
[04:19] <wgrant> Well, actually, once the WADL updates launchpadlib will probably tell them they're wrong before even sending the request
[04:19] <wgrant> But still, if they were relying on getting a 500 from that then they're pretty strange :)
[04:20] <StevenK> Let's find out in about an hour
[04:20] <wgrant> Hm?
[04:20] <wgrant> Why an hour?
[04:20] <StevenK> buildbot and qas updating
[04:21] <wgrant> Ah
[04:21] <StevenK> Current run has 15 minutes to go
[04:21] <StevenK> Unless wallyworld__ breaks it again
[04:21] <wallyworld__> it was only once :-(
[04:21] <StevenK> Twice
[04:22] <wallyworld__> once
[04:22] <wallyworld__> the other was suprious
[04:22] <StevenK> Once this morning and the feature flag related changes a few days ago
[04:22] <wallyworld__> same old thing we'vebeen seeing lately
[04:22] <wallyworld__> i thought you meant today
[04:37] <StevenK> Why did we add rf-setup-certs to LP?
[04:37] <wallyworld__> curtis said to
[04:37] <StevenK> I thought it was supposed to go into lp-dev-utils ?
[04:37] <wallyworld__> oops, maybe, let me check
[04:37] <wallyworld__> StevenK: the email said utilities
[04:38] <StevenK> Hmmm
[04:38] <wallyworld__> i can move it
[04:38] <StevenK> wgrant: What do you think?
[04:39] <StevenK> I think it should move
[04:40] <wallyworld__> wgrant: StevenK: +expirable-bugs oops on a dsp. i can't see a link so it must have been via url hacking. i guess it should return a 404 instead of a 500? or did we want to implement expirable bugs on a dsp?
[04:40] <StevenK> wallyworld__: So the ZCML has +expirable-bugs on IBugTarget
[04:41] <StevenK> A DSP is a bug target, but it doesn't support it
[04:41] <wallyworld__> ah. but there's no link on the dsp bugs page
[04:41] <StevenK> Curtis and William are of the opinion that it should support it
[04:41] <wallyworld__> ok, will look at that, thanks
[04:41] <wallyworld__> and then we can add the link to the page
[04:43] <wgrant> wallyworld__: I'm not really of the opinion that it should support it
[04:43] <StevenK> wgrant: Do you think Colin's work is far enough long to close bug 556839?
[04:43] <wgrant> I'm of the opinion that it might be easier to fix it to support it than it is to remove the page
[04:43] <wgrant> Given it's registered for IBugTarget
[04:43] <wgrant> StevenK: No
[04:43] <StevenK> We don't use async by default?
[04:43] <wgrant> wallyworld__: I'd put the cert gen script in lp-dev-utils, I think
[04:43] <wgrant> As StevenK suggests
[04:44] <wallyworld__> wgrant: why wouldn't we want to find expirable bugs on a dsp?
[04:44] <wgrant> wallyworld__: 'cause even Distribution:+expirable-bugs had only 136 hits last month
[04:45] <StevenK> Hah
[04:45] <StevenK> And how many of those were Google?
[04:45] <wallyworld__> hmmm.
[04:45] <wgrant> Most were probably search engines, yeah
[04:45] <wallyworld__> maybe we want to redo the zcml then to stop lying?
[04:45] <wgrant> +expirable-bugs was mostly introduced to show what would happen when expiration was turned on
[04:46] <wgrant> wallyworld__: Right, it's possibly best to just register it on IProduct and IDistribution
[04:46] <wgrant> Not sure if IProductSeries or IDistroSeries or IProject have it linked
[04:46] <wallyworld__> ok, i'll check that out. i think that's the better solution
[04:47] <wallyworld__> rather than doing something no one will use
[04:47] <wgrant> IProductSeries and IDistroSeries have about as many hits as I'd expect from Google
[04:47] <wgrant> But it's difficult to say
[04:47] <wgrant> wallyworld__: Right
[04:47]  * wallyworld__ first gets out the hedge trimmer
[04:47] <wgrant> In some situations like this it's a one-liner to fix it, so it's easier to just make it work
[04:48] <wgrant> But this is not one of them
[04:48] <wallyworld__> yeah, dicking eith _getTargetJoinAndClause() for dsp would be messy
[04:49] <StevenK> wgrant: So, for bug 1050191, I agree with Curtis. If it was in +junk first, we don't care.
[04:49] <_mup_> Bug #1050191: allocate-revision-karma.py is too slow to complete <branches> <karma> <lp-code> <oops> <Launchpad itself:Triaged> < https://launchpad.net/bugs/1050191 >
[04:49] <wgrant> StevenK: It's on my chopping block for the next few days
[04:49] <wgrant> As it's directly implicated in the BranchRevision drama
[04:49] <StevenK> Right
[04:49] <StevenK> I'll leave it alone then
[04:49] <wgrant> Fixing it is probably a prereq, but in which way I'm not sure
[04:49] <wgrant> Right :)
[04:50] <StevenK> Is mawson back to crying about your queries?
[04:50] <wgrant> No
[04:50] <wgrant> I have the data I need for now
[04:50] <wgrant> You can molest mawson if you so desire
[04:51] <StevenK> I have no need to, I was curious.
[04:51] <wgrant> Oh right, you qa-meh'd the PCJ thing
[04:51] <StevenK> I did, yes.
[04:51] <StevenK> Bug 1031751 is interesting. I've seen a test fail in that way too
[04:51] <_mup_> Bug #1031751: KeyError: 'primary_vars'  raised setting branch for a project <oops> <Launchpad itself:Triaged> < https://launchpad.net/bugs/1031751 >
[04:52] <wgrant> Right, some of Orange's work triggered that a week or so ago
[04:52] <wgrant> An unrelated cause, but similar exception
[04:52] <StevenK> Yeah. I'm not sure about the cause for the bug
[04:54] <StevenK> Almost at 45 revisions needing deployment
[04:54] <wgrant> StevenK: I suspect a branch was being created with a private team as the registrant
[04:55] <wgrant> StevenK: Perhaps registering a codeimport
[04:55] <wgrant> But the OOPS is gone and the traceback severely truncated
[04:55] <wgrant> Care you try what I suggested on qastaging?
[04:55] <StevenK> +new-import with a private team?
[04:56] <wgrant> That shouldn't set the registrant to the primary team, but it might
[04:56] <wgrant> So it's worth a try
[04:57] <StevenK> PrivatePersonLinkageError: Cannot link person (name=rhinos, visibility=PRIVATE) to <CodeImport for ~rhinos/launchpad/test-import> (name=None)<br />
[04:59] <wgrant> Indeed
[04:59] <wgrant> I can't see an obvious way to set the registrant to a private team
[04:59] <wgrant> And you can't log in as a team any more, so that won't work...
[05:00] <wgrant> Oh, hmmm
[05:02] <wgrant> That OOPS would make it obvious :(
[05:23] <lifeless> .oOo.
[06:01] <wgrant> StevenK: Ah, it would have helped if I'd read the summary
[06:02] <wgrant> OOPS-07a7ee485f0ae949f3e103cc1db1c082
[06:04] <StevenK> Nice.
[06:04] <StevenK> What was the cause, then?
[06:10] <StevenK> Hmmmm, I think I get it. Creating an import for a series owned by a private team
[06:11] <lifeless> is sso glitching
[06:11] <lifeless> I'm having trouble logging into oops.c.c
[06:12] <wgrant> lifeless: It's working for me
[06:13] <lifeless> I'm getting The connection was reset
[06:13] <lifeless> on
[06:13] <lifeless> https://oops.canonical.com/openid/+return?janrain_nonce= ...
[06:13] <wgrant> StevenK:                     code_import = getUtility(ICodeImportSet).new(
[06:13] <wgrant>                         registrant=branch_owner,
[06:13] <wgrant> From ProductSeriesSetBranchView.update_action
[06:15] <StevenK> wgrant: So we should croak in validate if branch_owner is private?
[06:16] <wgrant> StevenK: No
[06:16] <wgrant> You should specify the registrant
[06:16] <StevenK> Oh, registrant=self.user ?
[06:17] <wgrant> Yeah
[06:17] <StevenK> wgrant: But I thought the form was asking who should own the branch?
[06:17] <StevenK> So isn't that ignoring them ...
[06:17] <wgrant> own != register
[06:17] <wgrant> So owner != registrant
[06:18] <wgrant> I think you'll find that all the other branch creation callsites use the user as the registrant, with the provided owner as the owner.
[06:18] <wgrant> That's certainly the intent.
[06:19] <wgrant> I suspect that Registry got confused when producing this view
[06:19] <StevenK> Right
[06:19] <StevenK> Ugh, all of the tests for it are in doctests
[06:31] <StevenK>  I chose SHA-512 because everyone knows it's 512 times more secure than SHA-1.
[06:31] <StevenK> — Rusty Russell
[06:31] <wgrant> :)
[06:35] <StevenK>         obj_info["primary_vars"])
[06:35] <StevenK>     KeyError: 'primary_vars'
[06:35] <StevenK> Fantastic
[06:36] <wgrant> In a test?
[06:36] <wgrant> I wouldn't have a test for this issue directly.
[06:36] <wgrant> Rather amend an existing test to check that the registrant is the user, not the owner
[06:37] <wgrant> Making the branch approximately -1/+2
[06:39] <StevenK> But I just wrote one :-(
[06:48] <wgrant> StevenK: It may be useful for checking that the issue is really gone, but it's not something that's worth the test suite time and LoC to keep forever
[06:56] <StevenK> Raaaargh
[06:56] <StevenK> This entire test uses the product.owner as the registrant as well as the logged in the user
[06:57] <wgrant> Change one of them to be the other, I guess
[06:57] <StevenK> Changing the whole test to make a driver and set them as the owner of the series has lots of ('Invalid value', InvalidValue("token u'person-name-100001' not found in vocabulary"))
[06:57] <wgrant> StevenK: Ensure that the user is a member of the team
[06:57] <StevenK> What team
[06:57] <StevenK> ?
[06:58] <wgrant> You can only create a branch owned by yourself or a team in which you participate
[06:58] <wgrant> So the owner has to be a superteam of the user
[07:04] <StevenK> Right, large diff
[07:20] <StevenK> wgrant: Did you want to review?
[07:20] <wgrant> Sure
[07:21] <StevenK> wgrant: https://code.launchpad.net/~stevenk/launchpad/no-private-registrant-setbranch/+merge/128173
[07:26] <jpds> StevenK: Do you use ROT26 from time to time?
[07:27] <StevenK> ROT26 is the alphabet?
[07:43] <adeuring> good morning
[10:50] <rick_h_> morning
[10:54] <czajkowski> rick_h_: hey there
[10:54] <czajkowski> you're on early
[10:55] <rick_h_> czajkowski: trying to get back on time
[10:56] <czajkowski> rick_h_: can you pop into #lp please if you're free?
[11:00] <rick_h_> adeuring: ping, can you help czajkowski debug a blueprint issue? https://oops.canonical.com/?oopsid=OOPS-51c8161b2e479d7ed62ba67c1000bd19
[11:00]  * adeuring is looking
[11:01] <rick_h_> adeuring: looks like a user hit a forbidden setting it to embargoed or something.
[11:01] <adeuring> 383264
[11:04] <rick_h_> adeuring: ? typo'd bug number or something?
[11:06] <adeuring> rick_h_: PEBCAK: I tend to forget to click into a wondow before typing something. I this I wanted to add a 2-fact auth number into a browser window, while irc IRC window still had the focus...
[11:06] <rick_h_> adeuring: ah gotcha cool
[11:07] <rick_h_> thanks for looking at the oops
[11:41] <adeuring> rick_h_: https://bugs.launchpad.net/launchpad/+bug/1062207/comments/1
[11:41] <_mup_> Bug #1062207: Unable to raise blueprint <Launchpad itself:New> < https://launchpad.net/bugs/1062207 >
[13:22] <rick_h_> abentley: I missed you yesterday about setting the field value = PUBLIC
[13:22] <rick_h_> abentley: that was my doing when I setup the UI because I needed to set a value to the form field so that choicepicker stuff would pick it up
[13:22] <rick_h_> now that the field exists, model sets, etc it can probably just go away
[13:22] <deryck> adeuring, thanks for the review.  Agree on both counts.  We can hold the individual adapters until after alpha though, I think.
[13:23] <adeuring> deryck: right
[13:23] <abentley> rick_h_: I've removed it, and there seem to be no ill effects.
[13:23] <rick_h_> abentley: right
[13:23] <rick_h_> and the IProductSet checks were because there were tests that hit that View that weren't of that interface and weren't getting information_type added onto it
[13:24] <rick_h_> abentley: I had talked to deryck about that one when I thit it
[13:24] <abentley> rick_h_: Strange.  We don't seem to have any tests like that now.
[13:25] <rick_h_> abentley: it was something I hit. I had a chat with deryck about it asking if there was a better way than the interface checks
[13:26] <rick_h_> abentley: ProjectGroup perhaps gets hit in there?
[13:26] <abentley> rick_h_: I don't understand.
[13:28] <rick_h_> abentley: I'll bring it up on the call in a sec
[14:06] <abentley> After 3 years, I think I'm starting to understand buildbot's waterfall display.
[14:13] <jelmer> abentley: lol
[14:14] <abentley> jelmer: It's funny 'cause it's true.
[14:39]  * deryck switches work locations, back online shortly
[15:10] <sinzui> adeuring, bac, abentley: do either of you have time to review https://code.launchpad.net/~sinzui/launchpad/contact-new-maintainer/+merge/128273
[15:10] <adeuring> sinzui: i'll look
[15:24] <adeuring> sinzui: if the maintainer is a single person, not a team, user_address in LicenseNotification.send(), while it is a list of strings if the maintainer is a team. shouldn't a one-element list be used for "person is maintainer" case too?
[15:26] <adeuring> sinzui: never mind, i see now taht a single address worked before
[15:27] <sinzui> adeuring, indeed that is confusing. I had to read the code of our send mail helper to see that it would work
[15:29] <adeuring> sinzui: right, I an often a bit worried when a function(method argument can have different types. ANyway, r=me
[15:29] <sinzui> thank you adeuring
[15:36] <rick_h_> adeuring: branch for you if you get a sec, I noted a 'known issue' for the backend stuff that needs updating will be coming in a second branch. https://code.launchpad.net/~rharding/launchpad/edit_product_info_type/+merge/128081
[15:36] <adeuring> rick_h_: I'll look
[15:36] <abentley> adeuring: Do we have an equivalent for get_specification_privacy_filter that works for product listings?
[15:37] <adeuring> abentley: not yet...
[16:01] <adeuring> rick_h_: r=me
[16:07] <rick_h_> adeuring: ty much
[16:56] <deryck> rick_h_, abentley -- I'm going to have to leave here shortly, a little earlier than expected.
[16:56] <deryck> rick_h_, abentley -- but I'll obviously be around tonight and tomorrow as I finish tests and get my last branch landed.
[17:20] <rick_h_> deryck: rgr
[19:34] <abentley> rick_h_: I'm having trouble with a DB query-- getting duplicate results back.  Can you have a look at it with me?
[19:34] <rick_h_> abentley: sure thing
[19:35] <abentley> rick_h_: let's hangout
[19:35] <rick_h_> abentley: rgr
[19:59] <rick_h_> abentley or bac if you're still around. Would appreciate a review of https://code.launchpad.net/~rharding/launchpad/store_edit_product_info_type/+merge/128288
[19:59] <abentley> rick_h_: looking.
[19:59] <rick_h_> abentley: ty much
[20:04] <abentley> rick_h_: Where is _setLicense being invoked?
[20:05] <rick_h_> abentley: it's set by the ProductEditView when all the fields are updated.
[20:05] <rick_h_> abentley: there's an assumption there because changing the information_type changes the license I guess...
[20:06] <abentley> rick_h_: I think this change leaves a hole in the validation, because it allows information_type to change without setLicense being invoked.
[20:06] <rick_h_> abentley: yea, right
[20:07] <abentley> rick_h_: Can we do setLicense first?  Or add the subscription in _set_information_type?
[20:09] <rick_h_> abentley: thinking, really hate to call setLicense from set_information_type
[20:10] <abentley> rick_h_: Not set_license, but adding the subscription.
[20:10] <rick_h_> so maybe the best thing is to try to pull out the block that does the commerical sub generation and share it between teh two
[20:12] <rick_h_> what about a check in set_information_type that if the information_type is EMBARGOED/PROPRIETARY it sets the license manually there, overrides so that it makes sure it's getting the OTHER_PROPRIETARY value
[20:12] <abentley> rick_h_: That would make sense.  Either the license or the information_type could be seen as proprietary intent.
[20:12] <rick_h_> yea, it's just more a functino of license so trying to think of a good way to keep it there
[20:12] <abentley> rick_h_: r16102 provides make_product_form which should simplify your code.
[20:12] <rick_h_> the root issue is you can set the information type, and the license might not kick in.
[20:13] <rick_h_> abentley: cool, thanks for the heads up on that.
[20:14] <rick_h_> abentley: but can the license field ever not be supplied? It's a required field of data
[20:15] <abentley> rick_h_: The view code should never be responsible for the sanity of the data.  Especially given we have a web service API.
[20:16] <rick_h_> abentley: yea, sorry I thought it was required in the model via the validate method, but that's just worried about resetting the license review
[20:21] <abentley> rick_h_: So, I think the subscription is the key thing.  The license doesn't matter for information types.  A product could have an open source license, but not be announced yet.  Which is fine, as long as they're paying for the right to secrecy.
[20:22] <rick_h_> abentley: yea, so I'll pull that out into a new create_complimentary_subscription method and call from both places. I need to make sure it'll be safe to run twice
[20:22] <rick_h_> since it could get called via both the license setter and the information type setter
[20:23] <abentley> rick_h_: Sounds good.  We seem to be calling such methods "ensure_FOO", rather than create_ or make_.
[20:25] <rick_h_> abentley: not sure ensure fits here like it does fixing policies
[20:25] <rick_h_> it's just creating an initial compl. commercial sub
[20:28] <abentley> rick_h_: It's like it in the sense that it doesn't try to create things that already exist.
[20:32] <rick_h_> ok, fits my head strange but I can buy in :) Pushed an update to the MP when it processes
[20:33] <rick_h_> bah, I need to update the test though to verify the subscription exists as well
[20:37] <abentley> rick_h_: You don't need line 116 because it duplicates line 148.
[20:38] <rick_h_> abentley: ah true thanks.
[20:41] <abentley> rick_h_: I would do http://pastebin.ubuntu.com/1262684/ but I believe your code's equivalent.
[20:43] <rick_h_> abentley: thanks for the suggestino
[20:45] <abentley> rick_h_: r=me either way, but I'm surprised you don't have to change the view in order to solve the bug.
[20:46] <rick_h_> abentley: the view was in the first branch to get the field to show up
[20:46] <rick_h_> abentley: it's listed as a pre-req to this MP
[20:46] <abentley> rick_h_: Gotcha.
[20:47] <rick_h_> thanks for the late day review abentley
[20:47] <abentley> rick_h_: np
[20:47] <abentley> rick_h_: still before my EOD.
[20:48] <rick_h_> still, late Friday bonus reviews yay ...not :)
[20:54] <abentley> rick_h_: So thanks for the advice on debugging the privacy filter.  It turned out that the And() needed to be in a sub-select, because that Product._information_type == InformationType.PUBLIC evaluates true for most rows.
[21:00] <rick_h_> abentley: cool, glad it worked out. Is that grantee thing still an issue or was that nothing?
[21:03] <abentley> rick_h_: The grantee thing was not really part of it.  It was just generating rows where AccessPolicyGrantFlat.grantee_id != TeamParticipation.teamID but Product._information_type == PUBLIC, so such rows were being yielded in the result.