wgrantwallyworld_: Because the team has access, a grant is not created for the member.00:00
wgrantwallyworld_: So they only have access through the team.00:00
wgrantWhen the team's access is revoked from +sharing, a REmoveGranteeSubscriptionsJob removes the team's subscription.00:00
wgrantBut doesn't remove the person's subscription, even though they no longer have access.00:00
wallyworld_oh, from +sharing. the remove subscription for former members works when using the team page to revoke membership00:01
wallyworld_so there could be a case to drop the RemoveGranteeSubscriptionsJob and just rely on the RemoveBugSubscriptionsJob00:02
wgrantwallyworld_: I think they should indeed be merged.00:03
wgrantbut RemoveBugSubscriptionsJob would need to not require bug_ids.00:03
wallyworld_probably, it will need to be looked at00:03
wallyworld_the RemoveGranteeSubscriptionsJob was written first and then several pipes later the RemoveBugSubscriptionsJob came along00:04
wgrantAh, I see.00:05
StevenKPerhaps we should just use RemoveGranteeSubscriptionsJob and have it for both branches and bugs00:06
wallyworld_StevenK: that job was written to handle both00:07
StevenKThen why does RemoveBugSubscriptionsJob exist? :-)00:08
wallyworld_StevenK: the former removes access for an individual subscriber from the specified bug/branches for a specified pillar00:08
wallyworld_the later removes subscriptions for any artifacts any person can no longer see00:09
wallyworld_for any pillar00:09
wallyworld_the former was written early on to work will the +sharing page00:09
wallyworld_the latter was written to handle team membership revocation and evolved from there00:10
wgrantThe rules for both should be the same.00:10
wgrantBut one is information_type-wide00:10
wgrantAnd the other is restricted to a set of bugs.00:10
wgrantBoth need to consider teamparticipation.00:10
wallyworld_one is potentially more efficient because it is more constrained00:11
wallyworld_so perhaps both are needed, but the remove gants one needs to include team membership00:11
wgrantWhy are both needed?00:11
wgrantOne has bugtaskflat.information_type = 300:11
wgrantThe other has bugtaskflat.bug IN (1, 2, 3, 4)00:11
wgrantThey should otherwise be identical, AFAICT00:11
wallyworld_i did say "potentially" - i need to review the code00:12
wallyworld_the one from the +sharing page is constrained to that pillar so potentially may have less work to do in the db00:12
wallyworld_bugtaskflat.bug IN (1, 2, 3, 4) will return far less rows that infotype = 300:13
wallyworld_hence the downstream processing may be less00:14
wgrantEr yeah, I mean (bugtaskflat.information_type = 3 and bugtaskflat.product = 1) or (bugtaskflat.bug IN (1, 2, 3, 4))00:14
wgrantThe two types of constraints are required for efficiency.00:14
wallyworld_still less rows00:14
wgrantBut the implementation can otherwise be identical.00:14
wgrantJust one filters by infotype and pillar, the other by a predefined list of bug IDs.00:14
wallyworld_yes, the implementations can likely be merged now that both have been written00:15
wallyworld_but the different filtering should remain probably00:15
wgrantwallyworld_: btw, bin/celeryd -l DEBUG -Q job --config lp.services.job.celeryconfig00:38
wgrantHandy for testing stuff outside tests.00:38
wgrantWithout waiting 50s for the cronjob to start00:38
wallyworld_cool, i did know that a couple of weeks ago but forgot00:39
lifelessahahaha http://www.leakedin.com/01:21
lifelessalso leakedin.org :>01:25
wallyworld_StevenK: what did we decide about have a public A stacked on public B but then making B userdata - A should also become userdata?02:00
StevenKwallyworld_: Hm, I think I've swapped that out. Let me think.02:01
wallyworld_ok. i have a test which does that and fails02:01
wallyworld_it used to set private = true and the triggers would update everything02:02
wallyworld_but setting info type = userdata fails since A remains public02:02
StevenKwallyworld_: Right, so the information_type won't change, but we check if the stacked_on branches information_type is private in places where we were depending on transivitely_private.02:05
wallyworld_StevenK: sure, we now check for branches "under" the one being changed, but the old transitively private stuff also went "up" the chain02:06
StevenKYes, we don't do that.02:07
wallyworld_i think we need to02:07
wallyworld_or else we will have public stacked on user data and will be in the same mess02:07
wallyworld_as we had before transitively private02:07
wgrantwallyworld_: It's less than practical to do that universally, and the cases where it's useful are less than common.02:09
wallyworld_it's not impractical if we use triggers02:09
wgrantwallyworld_: If you're changing a stacked-on branch from public to private, you should know you're already in strife.02:09
StevenKNeither wgrant or I want to use triggers.02:09
wgrantwallyworld_: It's still impractical.02:09
wallyworld_why impractical?02:10
wgrantwallyworld_: It's just that it's so unused nowadays that the impractical cases haven't shown up yet.02:10
wallyworld_and if we don't fix this, users will shoot themselves in the foot02:10
wgrantwallyworld_: Writing to a few thousand branches because of a write on one branch is silly.02:10
wallyworld_that's what we do now with the triggers and it works fine02:10
wgrantwallyworld_: Users have shot themselves in the foot in the past due to Launchpad staff misconfiguring branch visibility policies.02:10
wgrantsinzui has since fixed them, and in the new model it's impossible.02:11
wallyworld_so are you sating allowing a public branch to stack on a private branch is ok?02:11
wallyworld_because the code as written allows that02:11
wgrantwallyworld_: The normal footgun situation is handled here, as it was before. If you push up a new public branch stacked on a private branch, it will be overridden to private.02:11
wgrantWhat's not handled is making an existing public stacked-on branch private.02:12
wallyworld_wgrant: sure, but now the other way arpund02:12
wgrantNow, we should probably warn in that case.02:12
wgrantBut it's going to be extremely rare.02:12
wallyworld_you can make a stacked on branch private02:12
wallyworld_the code allows it02:12
wallyworld_so people will do it02:12
wallyworld_there's a test which does it02:12
wgrantIt's possible to do.02:13
wallyworld_i think we are leaving a huge hole02:13
wgrant+sharing will say "you have n private branches"02:13
wallyworld_and don't understand why we are regressing02:13
wgrantThe public branch will probably have a warning say "wtf are you crazy"02:13
wgrantBut the situation should never occur again.02:13
wgrantUnless someone forgets to set up private branches before they push their code.02:13
wallyworld_should !+ won't02:14
wallyworld_it will and can occur via the api i think (need to check)02:14
wgrantI'm not saying that it's impossible to do.02:14
wgrantI'm saying that it is very unlikely to happen.02:14
wallyworld_the system should not allow people to do this02:14
wgrantUnless someone deliberately does it.02:14
wallyworld_unlikely != won't02:14
wallyworld_well they may not realise02:14
wallyworld_what's to stopme from changing my branch to private not realisng it is stacked on02:15
wgrantBut why would you do that?02:15
wallyworld_what, change a public branch to private?02:15
wallyworld_why not?02:15
wallyworld_you may want to do future work in secret02:16
wgrantAnyway. Doing unbounded cascading writes is foolish, and this case is sufficiently harmless and so extraordinarily uncommon that I'm pretty sure it's not worth it.02:17
wallyworld_it's not unbounded really02:17
wgrantThere may be some sense in adding a warning to the stacked branch.02:18
wgrantIt is unbounded.02:18
wallyworld_not in practice02:18
wallyworld_and it works just fine now02:18
wallyworld_so we are causing a deliberate regresiion02:18
wgrantHas it ever been used?02:18
wallyworld_in tests02:18
wallyworld_i have no way of knowing whether on prod02:18
wgrantIt wasn't used during the several week information_type transition on production.02:19
wallyworld_ingrequent != won't ever02:19
wallyworld_we should not allow the system to get into a bad state02:20
wgrantlaunchpad_dogfood=# SELECT COUNT(*) FROM branch WHERE stacked_on = (SELECT id FROM branch WHERE unique_name = '~launchpad-pqm/launchpad/db-devel');02:20
wallyworld_and that's what we wil be doing in introducing this regression in functionality02:20
wgrant count02:20
wgrant  501802:20
wgrant(1 row)02:20
wgrantThat's pretty unbounded.02:20
wgrantwallyworld_: What is wrong with the state?02:20
wallyworld_5018 < infinity02:20
wallyworld_public branch stacked on private branch doesn't make sense02:21
wgrantIt doesn't make sense, no.02:21
wallyworld_and yet we will be allowing it02:21
wgrantBut why do we need to prevent it?02:21
wallyworld_because it doesn't make sense02:21
wgrantWe should prevent things that are harmful.02:21
wallyworld_and the whole reason transitively private was done was to stop this02:21
wgrantNot quite02:21
wallyworld_that's the only reason transitivelt private was done02:21
wgranttransitively_private's main use case is handled by information_type02:22
wgrantThat case is the one where a branch is pushed as public, but is stacked on a private branch.02:22
wgrantIn that case the user is probably unknowingly making a mistake.02:22
wgrantWe need to handle that, because it happens often due to misconfiguration.02:23
wallyworld_and the other case will also happen02:23
wgrantThe case that transitively_private handles now, but we are dropping with information_type, is where a branch and its stacked-on are public, and the stacked-on transitions to private.02:23
wgrantIn that case the branch owner is already aware of some information disclosure.02:23
wgrantThey're going to want to audit their project's sharing once they've fixed the immediate hole.02:23
wallyworld_yes, and i totally disagree with that regresiion02:23
wallyworld_it is insane we are allowing bad data from one direction and stopping  it from the other02:24
wgrantWe're not doing it to stop bad data.02:24
wgrantWe're doing it to stop people from a footgun that they can't know about.02:24
wallyworld_and yet we will allow it still02:25
wallyworld_change the stacked on to private and boom02:25
wgrantBoom? What boom?02:25
wgrantNothing breaks.02:25
wgrantSome stuff may be left public.02:25
wgrantBut it's not unknown.02:25
wallyworld_what breaks is that people will want to work with the public branch but cant02:25
wgrantPublic->private of a stacked-on happens as a result of an incorrect configuration being discovered.02:26
wallyworld_because they can't see the stacked on private branch02:26
wgrantNow that's something we can't solve.02:26
wgrantThey can't work with it either way.02:26
wgrantIf it's public, they can no longer work with it since it's stacked on a private branch.02:26
wgrantIf we make it private, they can longer work with it since it's private.02:26
StevenK2012-06-06 23:26:57 DEBUG2  [PopulateSourcePackagePublishingHistoryPackageUpload] Done. 231995 items in 337 iterations, 807.064977 seconds, average size 688.415397 (287.45639486/s)02:26
wgrantSo it's no more unusable this way.02:26
wallyworld_yes, and so it should be marked private02:26
StevenKwgrant: ^02:26
wgrantwallyworld_: Why?02:27
wallyworld_so that people realise it's private. as i said, that why tp was introduced02:27
wgrantwallyworld_: This is not a regression which makes those forgotten branches less usable. And this is not a regression which prevents an audit from working, since the other branches will clearly show up as public too.02:27
wgrantwallyworld_: tp's main usecase is, as I said, to prevent the initial footgun case.02:27
wallyworld_it is a regression which removes previously agreed useful behaviour02:28
wgrant*Not* to handle this change later.02:28
wgrantIt's very expensive and probably not worthwhile behaviour.02:28
wallyworld_and we will now be allowing the initial footgun again02:28
StevenKwgrant: And racy.02:28
wallyworld_by allowing a stacked on branch to be marked private02:28
wgrantWe explicitly do handle the initial push case.02:28
wgrantThat's not the initial footgun.02:28
wallyworld_it's a way to create the problem without people knowing02:29
wgrantMaking a branch private later is a very explicit operation.02:29
wallyworld_and one which is easy to mess up02:29
wallyworld_if you don't realise yor branch is stacked on02:29
StevenKThen let's warn in the UI and move on02:29
wgrantAnd what is the fallout from it?02:29
wallyworld_we just simply cannot alllw this02:29
wgrantIt is messy, but not a significant security issue.02:30
wallyworld_ui != api02:30
wgrantSo a warning is probably sufficient.02:30
wallyworld_it's a significant functional issue02:30
wgrantIn an extraordinarily rare situation.02:30
wgrantWhere someone has already broken shit.02:30
wallyworld_since it inadvertantly stops peope working wit the public branch02:30
wgrantThat's irrelevant.02:30
wallyworld_not rare enough that tp wasn't needed02:30
wgrantWe'd just be advertantly stopping people from working with it.02:30
wgrantwallyworld_: tp solves the footgun case.02:30
wgrantThis also solves the footgun case.02:30
wallyworld_tp does more than that02:31
wgranttp's implementation happens to also solve the later transition case02:31
wallyworld_we are deliberately regressing02:31
wgrantThis does not. It consciously does not.02:31
wgrantBut it's not a substantial regression in any way.02:31
wallyworld_not "happens", it was a key part of the implementation02:31
wallyworld_it is a substantioal regrsiion02:31
wallyworld_we must not allow users to do things whuch break the system02:31
wgrantIt does not break the system.02:32
wallyworld_it breaks the ability for people other than themselves to work wit the stacked branches02:32
wgrantI can make a public branch unusable in other ways.02:32
wgranteg. by deleting the repository from it.02:32
wallyworld_2 wrongs doesn't make a right02:32
wgrantAnd all it does is avert a tiny bit of user confusion.02:33
wgrantSince if we follow tp, we just make the branch even less usable.02:33
wallyworld_it does a lot more than that02:33
wgrantNot more usable.02:33
wallyworld_why is the branch less usable wit tp?02:33
wgrantwallyworld_: Your complaint is that with information_type the branch is unusable by people who don't have access to the stacked-on branch.02:34
wallyworld_without tp, the ui lies02:34
wgrantwallyworld_: With tp the branch is just as unusable.02:34
wgrantBut the UI can say "uh, your branch is stacked on a private one, but it's public. you're probably wrong"02:34
wallyworld_but it the private branch could be several layers down02:34
wgrantThat's true.02:35
wallyworld_and model code also needs to treat the branch like it is private02:35
wallyworld_to exlcude it from search results etc02:35
wgrantIt doesn't break anything.02:36
wallyworld_if i change a staced on branch to private, any publiv branche sstaked on it must not show up in search results02:36
wgrantIt just makes it a little confusing.02:36
wgrantWhy not?02:36
wallyworld_because the stacked branch is to be considered private02:36
wallyworld_that was the required tp functionality02:36
wallyworld_when that work was speced out02:36
wgrantcanonical-payment-service and ztrustee were misconfigurations that sinzui fixed. information_type handles those identically, because it's the initial footgun case. libunity-webapps was a misconfiguration that required a full project audit afterwards. Everything else has less than 5 branches in this state.02:37
wgrantWhich proves that no large transition has ever made use of the tp ability that we're regressing on.02:37
wgrantExcept *possibly* libunity-webapps.02:37
wallyworld_sure, but just because it's uncommon, we can't just unilaterally decide to regress02:38
wgrantThat paste covers every branch that has made use of tp, except for those where explicity_private was set afterwards.02:38
wgrantwallyworld_: We're regressing on a feature that has never been used.02:38
wgrantNot once.02:38
wallyworld_what about libunity-webapps02:39
wgrantwallyworld_: What if we don't consider the stacked branch private?02:39
wgrantwallyworld_: What if we consider it misconfigured?02:39
wallyworld_and it's not so much that it's used, it is designed to prevent inconsistent data02:39
wallyworld_and confusion02:39
wgrantlibunity-webapps I shall not go into here, but it was a complete misconfiguration of a private project.02:39
wgrantIn the case of a misconfigured project like that one, you should be fully auditing +sharing anyway.02:40
wallyworld_even if we consider it misconfisgured, why would we deliverately design out a saftely check to prevent the misvonfiguration in the first place?02:40
wgrantwallyworld_: It's not proven useful even once, it requires unbounded write cascades, and it is non-trivial to implement with information_type.02:41
wallyworld_it was useful because of the state lp was in befre it was implemented02:41
wgrantThe main usecase for tp is extremely useful.02:42
wallyworld_there were a large number of public branches stacked on private02:42
wgrantThis one is not.02:42
wallyworld_and now we can have the all over again02:42
wgrantOnly if a project changes their stacked-on branch without touching the rest.02:42
wallyworld_why do you say no when we have already agreed it can happen?02:43
wgrantIn that case it's not a vulnerability.02:43
wgrantIt's a project being stupid.02:43
wallyworld_but they may no realise their branch is stacked on02:43
wgrantwallyworld_: As we've seen, the vast, vast majority of cases where it can happen is the initial push.02:43
wgrantSo we won't have a large number of public branches stacked on private.02:43
wallyworld_it can happen anytime02:43
wgrantWe might have 1% of what we had before02:43
wgrantwallyworld_: Technically it can.02:43
wgrantPractically it won't unless the project owner is a moron.02:44
wallyworld_and therefore it will02:44
wgrantIs it a problem?02:44
wallyworld_moron != careless02:44
wallyworld_otherwise tp would not have been approved to be resourced02:44
wgrantTP solved a very real problem02:44
wgrantIt was totally worth it02:44
wgrantIt also solved another problem that wasn't real.02:44
wgrantWe have hard data on that now.02:44
wallyworld_it was rel02:45
wallyworld_i can't recall the exact figures, but a garbo job was needed to clean up the data02:45
wgrantThat is entirely irrelevant to the nature of that data.02:45
wgrantThe volume and split are unrelated.02:45
wgrantThe data I just obtained from dogfood confirms my assumptions: that the case we're dropping is unused.02:46
wgrantIt's a case I was always suspicious about.02:46
wallyworld_used now != used ever02:46
wallyworld_and we are taking away a safety net02:46
wallyworld_to prevent bad data02:46
wgrantIt's not bad data.02:47
wallyworld_it is02:47
wallyworld_because it causes confusion02:47
wallyworld_and stuff to break02:47
wgrantPreventing bad data is not itself a useful goal02:47
wgrantThe "bad data" has to have some negative effect02:47
wallyworld_it does02:47
wgrantThe goal is to eliminate the negative effect.02:47
wallyworld_confusion, no longer being able to use the stacked branch02:47
wgrantSo let's stop talking about bad data, and start talking about the negative effects :)02:47
wgrantConfusion: granted, we can fix that with messages on the branch page, as we do already if the stacked-on location is invalid.02:48
wallyworld_or we could just prevent the bad data and not introduce a deliberate regresiion02:48
wallyworld_we would need to drill down the entire stack02:48
wgrantNo longer being able to use the stacked branch: that's a false statement; you can't use it if it's private either.02:48
wallyworld_but it's not pretending to be usable02:49
wgrantSo that goes back to confusion.02:49
wgrantWhich is very similar to the invalid stacked-on issue02:49
wgrantFor which we currently warn02:49
wgrantWith a warning on Branch:+index02:49
wallyworld_we never used to warn02:49
wallyworld_is that new02:49
wgrantWe've always warned.02:49
wgrantInvalid meaning not a valid URL, or the URL points to something that doesn't exist or is outside Launchpad02:50
wgrantNot meaning private02:50
wallyworld_nope, not in the case of public stacked on private afair02:50
wallyworld_we used to just show the stacked branch as if it were public02:50
wgrant12:49:24 < wgrant> Which is very similar to the invalid stacked-on issue02:50
wgrant12:49:27 < wgrant> For which we currently warn02:50
wgrant"is very similar to"02:51
wgrantNot "is"02:51
wallyworld_so it's new then02:51
wgrantThat warning is not new.02:51
wgrantExtending it to warn when the stacked-on is private would be new.02:51
wallyworld_but we never used to warn people their public branch was stacked on a private one02:51
wgrantI never said we did.02:51
wgrantI'm saying it's what we should do02:52
wallyworld_hence my assertion it will be new02:52
wgrantSure, warning *on privacy violation* is new.02:52
wgrantWarning on bad stacked-on in general is not.02:52
wgrantSo it's quite anomalous that we don't warn in the case of a privacy violation.02:52
wallyworld_even recusive stacked on02:52
wgrantI'm not sure about that.02:53
wallyworld_we may warn on bad stacked on, but perhaps not recusrively02:53
wallyworld_i guess it comes down to in part philosophy - should software prevent a known bad situation02:53
wallyworld_or should software regress deliberately02:54
wgrantIt's a tradeoff.02:54
wallyworld_given the current implementation works fine, i'm not sure i see why we are doing this02:54
wgrantWe can maintain messy code of unbounded computational complexity that will probably never be usefully used.02:54
StevenKwallyworld_: Are you done yelling at wgrant? :-)02:54
wgrantOr we can not.02:54
wallyworld_it's not messy02:55
wallyworld_the algorithm is very simple02:55
wgrantWith transitively_private it's not messy.02:55
wgrantWith information_type it is slightly moreso.02:55
wgrantBecause it's not boolean.02:55
wallyworld_so what i would like to see is an exception raised02:55
wallyworld_if someone attempts to change a stacked on branch to an invalid state02:55
wgrantWe can't do that every time.02:56
wgrantYou may recall we discussed this for weeks.02:56
wgrantThe information_type of a stacked on branch could change directly.02:56
StevenKstacking can change via an sftp client02:56
wgrantOr it could change by its stacked-on branch changing.02:56
wallyworld_sure, but i'm talking about public stacked on not public02:56
wgrantListen to this case:02:56
wallyworld_via sftp we can't detect sure02:57
wgrantI have a stacking chain. A stacked on B stacked on C02:57
wallyworld_but that's the case now with tp also02:57
wgrantThey're all public.02:57
wgrantNow, the owner of B changes it to stack on D, a private branch.02:57
wgrantWhat happens now?02:57
wgrantWe can't block that.02:57
wgrantSo A and B have to become private, or we have to let them be inconsistent.02:58
wgrantBlocking is the ideal solution.02:58
wgrantBut it's not feasible.02:58
wgrantSo we have to either propagate or accept.02:58
wallyworld_what happens now is A and B become private02:58
wallyworld_as they should02:58
wgrantThat is arguable.02:58
wallyworld_the argument was had before tp was approved02:59
wgrantAnd even if we decide that they should, it becomes is it worth it.02:59
wallyworld_and it came out in favour of tp02:59
wgrantCurtis agreed with my view on several calls, and I'm pretty sure you were there too :)02:59
wallyworld_i'm not sure it was agreed to intorduce a deleibate regression03:00
wgrantIt may have been agreed during TP's design on the absence of useful data.03:00
wgrantBut we have useful data now.03:00
wgrantIt was.03:00
wgrantIt was acknowledged as a regression.03:00
wgrantAnd I now have data which proves beyond reasonable doubt that nobody will miss it, because nobody has used it.03:00
wallyworld_it's not that nobody has used it i'm worried about - it's the ability to allow a misconfigured sysem, deliberately03:01
wgrantIt's less than ideal.03:01
wallyworld_because nobody uses it is even more a case not to allow it03:01
wgrantBut the misconfiguration is not fatal.03:01
wgrantThere are lots of other misconfigurations that we permit and can't reasonably forbid.03:01
wgrantAnd this misconfiguration is expensive to prevent.03:02
wallyworld_well we can reasonably prevent this one because we already do03:02
wgrantOnly in a way that will probably time out if it's ever used.03:02
wallyworld_nope, it's used now03:02
wgrantOn any project of significance.03:02
wgrantIt's *NOT*03:02
wgrantThe data I gave proves that.03:02
wallyworld_the triggers are run everytime a branch changes03:02
wgrantBut we're not talking about the triggers' overall behaviour.03:03
wgrantWe're talking about the particular aspect that we're dropping.03:03
wgrantThe unbounded cascade from the lower branch.03:03
wallyworld_i don't want to redo the cascade bit03:03
wallyworld_just the bit to prevent bad data03:03
wgrantThe bit we're regressing on is the cascade bit.03:04
wallyworld_to prevent changing a stacked on branch to userdata03:04
wallyworld_if the stacked branch is public03:04
wgrantNow, we could prevent that at the UI and API level.03:04
wgrantHowever, we can't prevent that at the bzr level.03:04
wallyworld_yes, that's all i am wanting03:04
wgrantOK, that is a more reasonable proposal.03:05
wallyworld_we don't do anything at the bzr level now for tp03:05
wgrantWe do03:05
wallyworld_wthat's all i've been arguing for here03:05
wgrantWhen the stacked-on changes via bzr, it will change in the DB.03:05
wgrantAnd transitively_private will work03:05
wgrantIt is no problem there because it's not trying to prevent the transition.03:05
wgrantJust react to it.03:05
wallyworld_ah right. well we should do the same check in the  new world also03:05
wgrantWe don't want to react to it.03:05
wgrantwallyworld_: Which is exactly the cascading thing.03:06
wgrantWe have three options:03:06
wallyworld_i'm not talking about cascading any changes as we do now, just now allowing the transiitin03:06
wgrant 1) Do nothing. Let people shoot themselves in the foot if they shuffle branches around in strange ways.03:06
wgrant 2) Cascade privacy changes through the tree.03:07
wgrant 3) Block transitions that violate the rules.03:07
wgrantTP does (2)03:07
wallyworld_i'm arguing for 3, and have been for 30 minutes03:07
wgrantinformation_type does (1)03:07
wgrant(3) is impossible, since we can't block bzr changes.03:07
wallyworld_why can't we block the stacked on change?03:08
wallyworld_does bzr write straight to the db?03:08
wgrantNo, which is exactly the problem.03:08
wgrantIt's FS access03:08
wallyworld_when in that case the triggers won't run03:09
wallyworld_so how are the stacked on changes propogated?03:09
wgrantThe branch scanner.03:09
wgrantAfter the push has completed.03:09
wallyworld_so the branch scanner could reject the change03:09
wgrantSo we cannot error at the time the client sets stacked-on03:09
wgrantReject how?03:09
wgrantWe could fail the scan.03:10
wgrantBut you'll never guess what that does.03:10
wallyworld_raise an oops?03:10
wgrantIt leaves the branch public, but puts a warning on its page :)03:10
wgrantWhich is exactly what I proposed.03:10
lifelesswgrant: you can block stacked on changes, just reject the IO operation03:10
wallyworld_leaves the stacked branch public?03:10
wgrantlifeless: lol03:10
wgrantlifeless: I don't feel like rewriting bzr lp-serve today03:11
lifelesswgrant: why would you need to rewrite it?03:11
lifelesswgrant: it has a lp VFS module already03:11
lifelesswgrant: I'm not saying you should, I'm disputing your assertion that its impractical.03:12
wgrantlifeless: If it's practical to do that (eg. due to restrictions around RPC from the lp-serve process), why wasn't that done from the start instead of doing it at scan time which is clearly inferior?03:12
lifelesswgrant: what restrictions, lp-serve makes RPC calls all the time03:12
wgrantlifeless: It's possible.03:12
wgrantIt's probably not practical.03:12
mwhudsonit's not scan time03:12
mwhudsonit's when the branch is unlocked03:13
wgrantmwhudson: branchChanged time, sorry03:13
wgrantAfter the end of the operation.03:13
mwhudson(which is admittedly too late to tell the writer to bugger off)03:13
lifelesswgrant: there was no initial requirement to patrol stacking03:13
lifelesswgrant: you are adding one, which implies change.03:13
lifelesswgrant: so why wasn't it done this way before, because it wasn't done before at all.03:14
wallyworld_and a change i think we should do03:14
wgrantlifeless: We break branches in LP when the stacked-on is invalid.03:14
wgrantThat's not new functionality.03:14
wgrantwallyworld_: Why?03:14
wgrantI'm pretty sure it's completely not worth the extra code.03:14
lifelesswgrant: what do you mean break them ?03:14
wallyworld_do i really have to explain myself all over again?03:14
wgrantAnd the data confirms it.03:14
wgrantIt's an extraordinarily rare situation.03:15
wgrantAnd it will become even rarer once people can't break private project setup.03:15
wallyworld_rare != we shouldn't prevent it03:15
wgrantNothing goes terribly wrong if we don't prevent it.03:15
wgrantAnd preventing it requires lots of extra code.03:15
wgrantdifficult to implement + very rare + largely inconsequential == not worth it03:16
lifelesswgrant: what do you mean break them ?03:16
wgrantlifeless: It stops scanning and throws warnings on the branch page.03:16
wgrantlifeless: Which is pretty unpleasant if it could just reject the push instead.03:16
lifelessit can03:18
lifelesshowever thats not break-the-branch, its aknowledge-that-its-broken03:18
lifelesswe're not altering the branch data are we?03:18
lifelesswgrant: wallyworld_: Can I ask, whats the context - you have a long and interesting discussion, but whats the core issue ?03:18
wallyworld_lifeless: we now handle the case if a stacked on branch is changed from public -> private03:19
wallyworld_any stacked branches will become private also03:19
wallyworld_but the new world will see use allow a user to change a stacked on branch to private will no warning and the stacked branches will remain public03:20
wallyworld_and the stacked branches will become unusable03:20
wgrantThe stacked branches will become unusable to unprivileged users, same as they would be if they were made private. The only issue is confusion if the branch says it's public but isn't usable.03:21
wallyworld_so we are allowing a footgun with no warning03:21
wgrantI gathered evidence from the database that this feature hasn't been used.03:21
lifelessso, I would say, that changing other peoples branches to private is a misfeature03:21
wallyworld_i'm not arguing it hasn't been used, i'm worried about the footgun03:22
lifelesswallyworld_: its fixable by the same user right ?03:22
wallyworld_ie allowing someone to do something dumb with no warning03:22
lifelessso the footgun specifically, is it:03:22
wallyworld_fsvo fixabe03:22
lifeless a) the user changes the privacy setting for the stacked-on branch to private03:23
lifeless b) in a public branch they set a stacked-on which is private03:23
wallyworld_we handle b)03:23
wallyworld_and it's a regresion because we handle a) now03:23
lifelesswith the general case of c) being 'setting a stacked-on which has a different visibility rule' (and applies if per-artifact access is given to a branch but not its stacked-on)03:23
lifelessdo we handle c)? how do we handle it ?03:24
wallyworld_transitively private03:24
lifelessI thought that column was deleted ?03:24
wallyworld_but it's complicated with info type03:24
lifeless(being deleted)03:24
wgrantlifeless: It's about to be deleted.03:25
wallyworld_yes, and so we need to figure out how to prevent the footgun03:25
lifelessso transitively private propogates outwards03:25
wallyworld_if you mean up the stack , yes03:25
lifelessa) is a propogates-outward case, c) is a propogates inwards case.03:25
lifelessoutward == up the stack, yes.03:25
wgrantinformation_type is currently independent, except that if you push a new public branch and it's stacked on a private one, then you're probably doing something stupid and disclosing previously private code, so we make it private.03:25
wallyworld_but we can't propogate info type03:25
wallyworld_we should just prevent bad things03:25
wallyworld_ie veto the info type change03:26
wallyworld_or stacking change03:26
lifelessok, so I'm much more worried about c than a.03:26
wgrantWith the propagation behaviour we're not disclosing previously private code.03:26
wgrantWe're just not automatically unleaking it.03:26
wgrantUm, with*out* the propagation behaviour03:26
lifelessa) very very few users will run into and they can restore functionality by unprivating the branch; or they can file a ticket to get a mass-migration of information type done (assuming they don't own all the branches)03:27
lifelessc) users will run into all the time, because feature branches are fairly common, and contractors are also fairly common03:28
wgrantThis is upsetting03:28
wgrantI don't get to argue with lifeless this time :(03:28
wallyworld_and c) is handled now but soon won't be03:28
lifelesswgrant: see, sometimes you are right ;)03:28
wallyworld_my view is that we should not allow the users to enter bad data03:28
wgrantIsn't (c) the case we *do* handle?03:28
wallyworld_if a user changes a branch to stack on a private one, the any public branches stacked on top "break"03:29
wgrantThat's case (a)03:29
wallyworld_with transitively private, they didn't03:29
wallyworld_a) is changing the privacy, not the stacked on03:30
wallyworld_or am i wrong03:30
lifelessah, I think the phrasing is poor.03:30
lifelesslet me rephrase.03:30
lifelessGeneral case there are three branch sets to consider.03:30
lifelessstacked-on branche(s) - its transitive following the pointers.03:31
lifeless<the branch>03:31
lifelessstacking-on-this branch(es) - anything that stacks on this branch, or transitively outwards03:31
wgrantwallyworld_: Ah right, misunderstood you.03:32
wgrantwallyworld_: Right, if the change an intermediate branch it still breaks.03:32
lifelesschanging the pointer in <the branch> to point to a branch with different visibility rules (e.g. different info type) is equivalent to to changing the visibility rule on a member of the stacked-on branch set.03:32
lifelessin terms of impact on the stacking-in-this branches.03:33
lifelessThe difference is whether its web ui (change info type) or bzr (change stacking pointer)03:33
lifelessfrom the perspective of any member of stacking-on-this, its identical.03:33
lifeless^ does this make sense ?03:33
wallyworld_we can detect that we are going to stack onto an incompatible branch03:34
wallyworld_but we are saying that we will not detect changing a branch which will cause outward brancvhes to become invalid03:34
lifelessand we can detect when changing info type that there is a branch stacked on this that is incompatible (again transitively)03:34
lifelessso, what user stories do we have around this03:35
wallyworld_but we the proposal is not to do that anymore03:35
lifelessone story is 'I have a branch that should be different info-type *and* it has stacked-on branches that I do not control'03:35
wallyworld_or 'has stacked on branches i do not know about'03:36
wallyworld_hence i may break them inadvertantly03:36
wallyworld_with no warning03:36
lifelessso what can we do here03:39
lifelesswe could stop the user changing the setting (be that stacked-on *or* information-type = we've established identity)03:39
wallyworld_so with transitively private, we allowed the change because it was transparently propogated03:40
wallyworld_but now it won't be03:40
lifelesswe could allow the branches that are no longer compatible [for users that don't meet the union of visibility rules] to become inoperable for those users03:40
lifelesswe could overlay the new information type into all the affected branches [which is what transitively private did]03:41
wallyworld_with allowing the branches to become inoperable, i'm arguing we should warn the user first03:42
lifelesshere is my suggestion03:42
wallyworld_via the api, not sure how to handle03:42
lifelessI think you should just do the change requested.03:42
lifelesstell the user the *impact* of the change.03:42
lifelessThey can switch it back in a heartbeat if they made a mistake, API *or* web UI.03:43
lifelessor bzr for that matter.03:43
wallyworld_that works i think, at least then it's an informed decision, not something done that they dont know about, and that was my main issue03:43
lifelessI totally agree that silent breakage at a temporal distance from the act is terrible03:44
lifelessthis will avoid that; we can, if you desire, send a warning to bzr too.03:44
wallyworld_yes, and this is what i was arguing against03:44
lifelessI think though, that anyone doing bzr set-stacked manually, is in a class of their own : I wouldn't bother handling it.03:44
lifelesswallyworld_: this == 'silent breakage at a temporal distance from the act is terrible' ?03:45
lifelesswgrant: ^ what do you think of my proposal?03:45
wallyworld_i have no problem with the new limitations in behaviour03:46
wgrantWe can't forbid, but we can warn on the branches at both ends, which is my preference.03:46
wallyworld_but the 'silent' breakage03:46
wallyworld_we can forbid it, no?03:46
wallyworld_sorry, i mean warn03:46
lifelesswallyworld_: so, the silent-breakage via bzr case is excruciatingly rare03:46
wallyworld_after they have done it03:46
lifelesswallyworld_: bzr makes it super hard to change03:46
lifelesswallyworld_: and lp sets the default behavior (to the branch of the default series)03:47
wallyworld_no problem there. but we can reasonably handle the other cases03:47
wallyworld_and i would argue that peope doing it via bzr are more expert anyway03:47
lifelesswarning in the web ui is easy; the API can return something with a warning too I expect.03:47
wgrantThe API can't return a warning that people will see.03:48
wgrantBut in the web UI we can.03:48
wallyworld_in the ui, warn as it is done as also after wards03:48
lifelesswgrant: OTOH folk doing API scripting are probably changing every branch they have.03:48
lifelesswgrant: e.g. the warning would be superfluous03:48
wgrantwallyworld_: Well03:48
wgrantwallyworld_: The branch edit page returns to the branch index page on submission.03:48
wgrantwallyworld_: So a bright warning at the top of the branch index page serves both purposes :)03:49
wallyworld_we can ask for a confirmation03:49
lifelesswallyworld_: nix on that03:49
wallyworld_in the web ui03:49
wgrantI don't think we should.03:49
wgrantIt's trivial to revert03:49
lifelesswallyworld_: confirmations for things that are easily reversible is an antipattern of usability.03:49
wgrantAnd more expensive to add confirmation03:49
wgrantIt'll be a matter of clicking the edit icon03:49
wgrantand clicking Public in the picker03:49
wallyworld_with the api, we could introduce a 'i really want to do this parameter'03:49
wallyworld_so it raises an exception unless that parameter is set03:50
wallyworld_that way, it's an informed choice03:50
wallyworld_since that is a case of breakage from a distance03:50
StevenKwallyworld_: Can you peer at http://pastebin.ubuntu.com/1027943/ ?03:52
lifelessexpose the attribute as readonly, use an API call to set it rather than direct mutation, and have the return value describe the impact.03:52
lifelesswallyworld_: ^03:52
wallyworld_lifeless: that works for me03:52
wallyworld_so long as the impact is communicated03:52
lifelesswallyworld_: wgrant: making a branch *public* that wasn't before is something that you might justify a confirmation for.03:53
wallyworld_StevenK: looking03:53
lifelessbecause unhiding previously hidden data is infeasible03:53
wgrantlifeless: Sure.03:53
wgrantlifeless: But that is precisely the opposite of this case.03:53
wgrantlifeless: That one is a general issue with the information type picker, unrelated to stacking.03:53
lifelessactually, its tightly related :P03:53
lifelesswhich is that fixing a public->private change involves a private->public change :)03:54
wgrantBut that's barely relevant.03:54
wgrantIt's a special case that will be fixed by the general fix.03:54
wgrantThere's nothing even really special about it :)03:54
StevenKLots of test fallout of forbidding open/delegated teams to subscribe to private branches. :-(03:55
wgrantWe really need to split the two types of teams up better.03:56
wgrantSo they're more obviously different types of objects.03:56
StevenKI think ec2 has found most of them03:56
wgrantmakeTeam and makeBadgeSeekersGroup03:56
StevenKOh, of course it has, the instance has been up for 4:0403:56
StevenKwgrant: Some of the fixes are trivial03:57
wgrantAll of them should be.03:57
StevenKtest_branchnamespace breaks in strange ways03:57
StevenK(When I make the 3 teams MODERATED)03:57
wallyworld_StevenK: i'm not sure what caused that error ottomh03:57
wallyworld_i'd need to debug it03:58
wgrantStevenK: What's the diff?03:58
StevenKwgrant: https://code.launchpad.net/~stevenk/launchpad/branch-subscribe-aag/+merge/108881 is the MP03:59
wgrantStevenK: So, the branch owner is trying to subscribe a private team that they can't see.03:59
wgrantI'm surprised that ever worked.04:00
wgrantThe test is clearly on crack.04:00
StevenKThere are a *bunch* of them04:00
wgrantReplace person_logged_in(...) with admin_logged_in()04:00
wgrantProblem solved04:00
wgrant(the write permission is not relevant in that test)04:00
* wallyworld_ has to do school pickup04:01
StevenKIt isn't relevant to check that the person subscribing can see the team?04:01
wgrantStevenK: Look at the test.04:01
wgrantStevenK: It is testing that an anonymous user can't see a private subscriber.04:01
wgrantNot that a private subscriber can or can't be created.04:01
wgrantSo the principal behind the creation is irrelevant.04:02
StevenKwgrant: That issue can't affect prod?04:03
wgrantStevenK: Only if someone is subscribing someone that they don't have View on, which I don't think is allowed since it discloses the team name.04:04
wgrantBut it's possible that someone with LimitedView can subscribe, I suppose.04:05
StevenKIt will oops04:05
StevenKSo we can find out04:05
StevenKRight, 18 failures, let's see how few I fixed04:05
StevenKHmmmmm. A test creates a private team and then creates a branch with that owner and it blows up too.04:09
wgrantStevenK: The branch owner is irrelevant.04:10
wgrantStevenK: The current principal is important.04:10
StevenKIt just calls into the factory twice, I'm not sure which principal is involed.04:11
StevenKwgrant: I've thought a problem -- create a project and have an open team as a reviewer. Create a private branch and propose it. Boom.04:20
wgrantStevenK: Reviewers aren't subscribed unless the branch is private.04:21
wgrantAnd that will largely go away now we have APGs.04:21
StevenKwgrant: Yes, and you can't subscribe an open team to a private branch ...04:22
StevenKAnd if the reviewer is an open team ...04:22
wgrantStevenK: Right, that's stupid and should be forbidden :)04:23
wgrantIt doesn't make sense.04:23
wgrantForbidding something that shouldn't be permitted is not a bug :)04:23
StevenKYes, so I was going to chase that rabbit hole for a bit.04:23
StevenKTrying to figure out where reviewers are in the DB04:24
StevenKHmmm, can't see it in the UI either currently, that's disappointing04:33
wgrantWhat do you mean?04:33
StevenKwgrant: So when I propose a branch for LP, a team is set as the reviewer. I'm trying to figure out where that is in the DB.04:34
wgrantStevenK: branch.reviewer04:35
StevenKI discounted that because I thought it was per-branch04:36
wgrantIt is per-branch.04:37
wgrantThe default reviewer for an MP is the target branch's default reviewer.04:37
StevenKWhich is in .reviewer for that branch, right.04:38
StevenKwallyworld: ^ Distracted from my other branch by a different problem.04:54
StevenKPeople that have set an open team as a reviewer need to be shot.04:55
wallyworldStevenK: i think so04:55
StevenKwallyworld: About which?04:56
wallyworldthe default mp reviewer is the branche's default reviewer04:56
lifelessStevenK: again, its our oversight05:50
lifelesss/again, //05:50
StevenKlifeless: Which is?05:50
lifelessfailing to consider the implications of open teams on integrity05:52
StevenKI'm going to bring it up on the call tomorrow.05:53
lifelessStevenK: you're aware of all the other cases we've fixed so far?06:00
lifelessStevenK: we should probably just do an audit06:00
lifelessppas, project roles, ....06:00
StevenKlifeless: I was involved in both of them closely.06:03
StevenKlifeless: However, these situations are not as cut-n-dried as PPAs and project roles.06:05
StevenKThere are arguments both ways, which there wasn't for the two earlier cases.06:06
wallyworldwgrant: https://code.launchpad.net/~wallyworld/launchpad/sharing-job-fflag-protection/+merge/10907206:12
lifelessStevenK: well, there weren't for ppas, because sudo on machines, but the rest there are :)06:50
lifelessStevenK: if you consider tarmac, this case becomes roughly === ppas06:50
wgrantlifeless: branch.reviewer is different, since most projects don't use Tarmac, so there are uses where an open team doesn't mean root for everyone.06:55
wgrantlifeless: For PPAs there's no use that doesn't mean root for everyone.06:56
wgrantwallyworld: That's the third feature flag for the same thing.06:57
wallyworldnot really06:57
wallyworldi reused one06:57
wgrantwallyworld: All the other stuff is guarded by one of the two feature flags that you introduced two days ago.06:57
wallyworldfor this06:57
wgrantAnd, critically, we don't want to turn writing on before we confirm that these work on production.06:57
wallyworlddifferent semantics06:58
wgrantSo turning this on only when we turn writing on is less than ideal.06:58
=== almaisan-away is now known as al-maisan
wgrantI'd have all the job creation under a flag like disclosure.that_job_stuff.enabled06:58
wgrantRather than 3 with misleading names.06:58
wallyworldto whom?06:59
wallyworldfor this branch, i just wanted a fast way to disable the jobs06:59
wallyworldso we can deploy06:59
wallyworldwithout having to add another flag07:00
wallyworldthe closest existing flag is perhaps disclosure.legacy_subscription_visibility.enabled but that one is for when we want unsubscribing to revoke access. the jobs are for the opposite direction so different07:02
StevenKlifeless: Tarmac and BMQs are a compelling argument indeed.07:02
wallyworldwgrant: so which flag did you want me to use if you disagree with the one i chose. remember i just want something quick to stop the jobs running on prod07:09
wgrantwallyworld: I'd probably just use disclosure.legacy_subscription_visibility.enabled. But I guess we use writable temporarily just to disable it, you're right.07:13
wallyworldi can clean up next branch07:13
wallyworldi just want to unblock deployment07:13
wallyworldwhich has been blocked for over a day :=(07:13
wgrantwallyworld: I'm on a call right now, but is that all the callsites for both types of jobs?07:14
wallyworldwgrant: unless i missed anything but i double checked07:15
wallyworldthere are 2 places in the service but the methods they are inside are protected07:16
wgrantwallyworld: r=me, thanks.07:16
wallyworldthank you07:16
=== al-maisan is now known as almaisan-away
lifelesswgrant: can you remind me tomorrow to send a summary to l-dev ?09:04
wgrantlifeless: Sure.09:06
lifelessalso ahahaha https://lp-oops.canonical.com/oops.py/?oopsid=61493187d8286292a72f12956de8b3cb#longstatements09:06
lifelessbranch fti query strikes again09:07
lifelesson +addseries09:07
* czajkowski peers at wgrant are you doing stuff to lp atm 09:07
wgrantlifeless: Ew09:07
lifelesselmo: ^ may make you laugh, may make you cry09:07
czajkowskihttps://lp-oops.canonical.com/oops.py/?oopsid=61493187d8286292a72f12956de8b3cb  am getting these while doing translations imports09:07
=== almaisan-away is now known as al-maisan
jmlI'd like to QA https://bugs.launchpad.net/launchpad/+bug/100629510:23
_mup_Bug #1006295: Archive's 'suppress_subscription_notifications' attribute is called 'commercial' in the database <qa-needstesting> <tech-debt> <Launchpad itself:In Progress by jml> < https://launchpad.net/bugs/1006295 >10:23
jmlI *think* the right thing to do is to set the attribute on an archive on qastaging and then perhaps check that 'commercial' was set in the database10:24
jmlcan someone confirm?10:24
wgrantjml: I'd just check that you can set it and read it.10:28
wgrantIf it reads back correctly, it probably wrote to the database, and probably to the commercial column :)10:29
jmlI guess I'll just use one of my own crappy testing PPAs10:29
jmlPython sucks.10:35
jmlLet's rewrite everything in Clojure.10:36
wgrantGood plan.10:36
jmlYou know what else sucks?10:36
jmltransparent network operation syntax10:36
jmlneither suppress_subscription_notifications nor commercial appear in the PPA dict for me (for a PPA I own)10:42
wgrantjml: Are you sure you're using the right API version?10:43
jmlwgrant: no.10:43
wgrantIt's probably only exported in devel, not 1.010:43
wgrantLaunchpad.login_with('fooconsumer', 'qastaging', version='devel')10:44
jmlwgrant: thanks.10:45
jmlwgrant: hmm. no change.10:45
wgrantjml: Perhaps you need to remove your WADL cache10:52
wgrantIn ~/.cache/launchpadlib or ~/.launchpadlib10:52
jmlwadl. cache.10:53
* jml parties like it's 199910:53
wgrantIt's on https://api.qastaging.launchpad.net/devel/~wgrant/+archive/ppa, so the server knows about it...10:53
jmlgetPPAByName doesn't work the way I think it should.10:57
jmlOr maybe createPPA doesn't.10:58
wgrantjml: Are you calling lp.me.somemethod?10:58
jmlwgrant: yes.10:58
wgrantThat doesn't work10:58
wgrantIt'll just return the person instead.10:58
jmlI'd noticed.10:58
wgrantwill work10:58
wgrantIf you're not dead yet.10:59
jmlwgrant: sanity check this script for me?11:01
jmloutput: http://paste.ubuntu.com/1028404/11:01
jmlis there a way I can get an alert when a revno gets deployed?11:07
wgrantjml: I can comment in the bug.11:09
jmlwgrant: thanks.11:09
wgrantBut probably in about 13 hours.11:09
=== matsubara is now known as matsubara-lunch
stubwgrant: Did you see anything horrible happen with the GIN index on staging?11:35
wgrantstub: It seems to not be broken.11:37
stubMight do the rest of them then11:37
=== rick_h changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2
=== matsubara-lunch is now known as matsubara
jmlcan someone in ~launchpad please claim a number for me in lp:~launchpad/+junk/dbpatches13:10
rick_hlooking jml13:11
jmlrick_h: thank you.13:12
rick_hjml: looks like I need a comment for it?13:12
jmlrick_h: Rename Archive.commercial to Archive.suppress_subscription_notifications (bug #1006295)13:13
_mup_Bug #1006295: Archive's 'suppress_subscription_notifications' attribute is called 'commercial' in the database <qa-ok> <tech-debt> <Launchpad itself:In Progress by jml> < https://launchpad.net/bugs/1006295 >13:13
rick_hjml: ok, think I got that13:18
jmlrick_h: thanks.13:19
jmlI got a lot of spew when I ran 'make newsampledata': http://paste.ubuntu.com/1028563/13:22
jmlmostly of the form, "NOTICE: there are circular foreign-key constraints among these table(s)"13:23
jmlis this my fault?13:23
jmlAlso, there appear to be changes to ArchivePermission and SPPH after running 'make newsampledata'. I wasn't expecting those. Should I not be doing this from db-devel?13:28
rick_hsorry jml, no idea there. Have yet to do a db patch myself. stub is the master, you know?13:28
=== jcsackett changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: jcsackett | Firefighting: - | Critical bugs: 3.47*10^2
rick_hoh hai jcsackett13:31
jcsackettmorning, rick_h.13:32
rick_hmorning, -ops13:32
* jcsackett idly wonders if the 1400 line diff in the MP queue is a harbinger of the day to come.13:32
* jml sends an email13:39
stubjml: You can ignore that I think. That make sampledata stuff is an old hack and has never been pretty.13:48
jcsackettsinzui: i need a refresher from last night. which of the many cards was the one wallyworld was looking at you suggested i try for today? my mind is sort of mush, evidently.15:02
sinzuijcsackett, https://bugs.launchpad.net/launchpad/+bug/99959615:03
jcsackettsinzui: ah, dig. yes, i should totally be able to fit that in today.15:03
jcsacketthuh. looks like william is already doing it though.15:04
sinzuijcsackett, I think the complication is that the vocabs and personset have competing implementation15:04
sinzuijcsackett, that is my mistake. No one is working on it15:05
jcsackettsinzui: dig. i'll assign myself then.15:05
cjwatsonoww, soyuz/doc/archive.txt makes me feel physically ill15:06
cjwatson2762 lines of solid doctest15:06
sinzuicjwatson, I bet half of it is tested in a unit test, but removing the duplicate parts topple the doctest15:08
cjwatsonAnd so it requires actual thought to dissect.  Grr15:09
* jml wants to take a swipe at it.15:12
cjwatsonI'm at -253 on it so far15:13
cjwatsonThough I strongly suspect there'll still be some left when I'm done15:13
bdmurrayCould I get my merge proposal review? https://code.launchpad.net/~brian-murray/launchpad/bug-912137/+merge/10904515:14
jcsackettbdmurray: i'm in the middle of running to somewhere that has better internet than my currently failing home spot. i'll check it as soon as i'm moved.15:15
bdmurrayjcsackett: okay, thanks!15:16
=== al-maisan is now known as almaisan-away
jmlstub: https://code.launchpad.net/~jml/launchpad/db-rename-archive-commercial-to-suppress/+merge/10917916:03
stubjml: The horrible hack to allow both the old and new column name has landed?16:06
jmlstub: indeed it has.16:06
jmlstub: and appears to work.16:06
stubI feel dirty16:06
jmlstub: it's not deployed yet.16:06
stubr=stub, with a comment that the code needs to be deployed before the patch can go live.16:08
stubEasiest way to guarantee that is to not land it until after deployment.16:08
jmlstub: that'll probably happen, since I need lifeless's patch before landing anyway, right?16:09
stubjml: No, you only need one of our approvals.16:09
jmlstub: ah ok. I'll wait until after I hear about the deployment then.16:11
stubThat would be FDT tomorrow I guess16:11
cjwatsonNDT, wouldn't it be?16:12
stuber, yer16:14
jmlsorry, I'm not familiar with those acronyms.16:14
stubfast downtime, no downtime16:14
cjwatsonfastdowntime -> db deploy, nodowntime -> code deploy16:14
jmloh right. sorry.16:14
jmlyeah, NDT16:15
jmlstub: oh, the patch number ends with -0. Is that a problem?16:26
stubNo problem. Numbers currently have no meaning besides ordering and uniqueness.16:26
jml stub: oh, the patch number ends with -0. Is that a problem?16:27
jmlstub: ah ok. cool.16:27
jcsackettbdmurray: are you still around? took me a good bit longer to get setup in new location than i anticipated, but i've looked at your MP and have some comments/questions.16:30
bdmurrayjcsackett: yes still here16:33
jcsackettbdmurray: awesome. so, your modification does change the sort order, but it has the side effect of putting any user that is subscribed via the "Subscribe someone else" button at the bottom of the list. I *think* this is probably why prepend was being used--so in long subscriber lists a user could still see the person they added was in fact added.16:34
bdmurrayjcsackett: ah, how did you go about determining the side effect?16:35
jcsackettbdmurray: i just used the "subscribe someone else" button above the subscribers list. given the change was commented as "add user to start of list" i wondered if that wasn't intentional.16:36
bdmurrayjcsackett: oh right, running it makes sense.  Do you any suggestions are where to go from here?16:37
jcsackettbdmurray: i do. i think you can probably update addSubscriber to take an argument named, say "add_new" that tells addSubscriber whether it's adding something being loaded from a list, and should be appended, or is adding a brand new person from the UI, and should be prepended.16:39
jcsackettyou could default that to 'false' and then just update the call that's bound to the "Subscribe someone else" button to set it to true.16:40
bdmurrayjcsackett: okay, thanks I'll have a go at it in a bit16:40
jcsackettbdmurray: awesome. i'll update your MP with these notes. ping me if you run into any problems--i'm happy to help. :-)16:40
=== salgado is now known as salgado-lunch
jmljames_w: I've ec2 landed your branch17:21
james_wI hope it stays landed17:21
=== matsubara is now known as matsubara-afk
sinzuijcsackett, do you have time to review https://code.launchpad.net/~sinzui/launchpad/unlock-sprite/+merge/10920618:17
jcsackettsinzui: i just opened the MP, actually. :-)18:17
sinzuisorry about the svg18:17
jcsackettno worries.18:21
jcsackettsinzui: r=me. looks good, thanks. :-)18:23
bdmurrayI forget what is username / password for launchpad.dev?18:24
bdmurrayjcsackett: ^?18:32
rick_hbdmurray: you can use mark@example.com with test18:35
bdmurrayjcsackett: I've updated my branch19:09
* jcsackett looks19:11
jcsackettbdmurray: r=me. do you have landing rights, or do you need an assist?19:22
bdmurrayjcsackett: its been a long time, so I could use some handholding landing it.  I mean I'd like to see if I still can.19:23
jcsackettbdmurray: dig. well, first off, are you set up for ec2 test/land? (https://dev.launchpad.net/EC2Test)19:29
bdmurrayjcsackett: yes, I should be set what is the proper comamnd to test and land?19:44
jcsackettbdmurray: If lp-dev-utils is on your path you just use "ec2 land".19:47
sinzuibdmurray, that would be "ec2 land https://code.launchpad.net/~brian-murray/launchpad/bug-912137/+merge/109045"19:51
sinzuijcsackett, are you available for a pre-imp talk. I am trying to teach Lp that projects can go uncommercial19:53
jcsackettI am actually in a coffee shop since wifi died at home. I don't think I could hear you over background noise.19:53
jcsackettI am told wifi should be restored in about an hour?19:54
jcsackettWell, Internet, not wifi.19:54
bdmurraytests are faster now right?19:58
sinzuibdmurray, yui tests are, the suite itself takes 6 hours and must be run on ec2, the buildbot twice19:59
sinzuibdmurray, expect a day for it to arrive on qastaging19:59
* sinzui looks at diff20:00
sinzuibdmurray, I suggest an alternate plan since you fixed a js-only issue20:01
sinzuibdmurray,  run ./bin/test -vvc --layer=YUI20:01
sinzuiif that layer passes, we can submit your branch directly to PQM20:02
bdmurraysinzui: I'm not in a hurry and want to make sure my ec2 setup is good so I think I'll let it run20:03
jcsackettsinzui: i am home, and i appear to have full connectivity. do you still want to chat, or have you moved on?21:01
sinzuioh, fab, yes lets talk21:01
jcsackettsinzui: i've started a hangout.21:04
=== salgado is now known as salgado-afk
=== jcsackett changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!