subscribing to a branch with "don't send diffs" does send diffs

There is history with this (as there is with any large system).

The diff size limits originally only applied to the diffs generated for revision email. When code reviews were added, not providing the diffs was not considered. There are several places in the code where we subscribe users with no email notifications for revisions or metadata, don't send diffs, but send all code review email.

These subscriptions would then change as the diff would no longer be sent.

Perhaps this is just a "notify the users" type update.

We could provide the link to the diff in the outgoing email if we don't send it.

Not technically difficult, but niggly edge cases.

In the end, we decided a simpler implementation would be to just not attach diffs if the subscription is status only. This avoids any surprises for people that have subscribed to full code-review email with no diffs.

It looks like my discussion with Peter Matulis via the support incident website, didn't reach the Launchpad team; I'll paste it here - the bug doesn't look like fixed.

03/09/2009 00:26 | Peter Matulis
Guilhem, changes have been made. Feel like conducting a test?

03/09/2009 19:41 | Guilhem Bichot
So I today subscribed myself to branch https://code.launchpad.net/~mysql/mysql-server/mysql-5.1, with those options in
https://code.launchpad.net/~mysql/mysql-server/mysql-5.1/+edit-subscription :
- Branch attribute notifications only
- Don't send diffs
- Status changes only
and then I proposed lp:~mysql/mysql-server/mysql-6.0-bugteam for merging into this branch (you can see it at
https://code.launchpad.net/~mysql/mysql-server/mysql-5.1/+merges
) and this time I didn't receive any email :-(
Whereas I used to receive a mail (but which had a diff, which was not ok, and was the subject of this support incident). No email is not better, alas.
What should I do, to receive a mail, without a diff?
Thanks!

or at least it didn't look like fixed when Peter asked me on Sep 3rd to make a test after changes had been made.
Let me know if it has been fixed since then and I should test again. Thanks.

On Fri, 18 Sep 2009 21:31:19 GuilhemBichot wrote:
> or at least it didn't look like fixed when Peter asked me on Sep 3rd to
> make a test after changes had been made. Let me know if it has been fixed
> since then and I should test again. Thanks.
>

Try again.

Hello. Wanted to try today. I go to
https://code.launchpad.net/~mysql/mysql-server/mysql-5.1/+merges, click on "view proposal details" for the 6.0-bugteam merge proposal, then it times out "Sorry, something just went wrong in Launchpad.". It has been happening repeatedly for 12 hours.

Tried a few minutes ago: subscribed to lp:mysql-server (same options as in previous posts), proposed a branch for merging (6.0-bugteam), and received only this email:

Launchpad encountered an internal error during the following operation: notifying people about the proposal to merge lp:~mysql/mysql-server/mysql-6.0-bugteam into lp:mysql-server. It was logged with id OOPS-1363MPCJ2. Sorry for the inconvenience.

I'm investigating.

The oops that you got there has to do with diffstat generation, and isn't related to the sending of the attachments.

Do you have a different branch that you can test with while I fix this?

I've manually grabbed the two branches you mention above, and I killed the merge --preview after it took over an hour and generated more than 33 meg of diff.

Is this normal for mysql?

Yes, 33 MB looks possible because 6.0 is a much newer version than 5.1.
You can try a different smaller merge:
propose https://code.launchpad.net/~mysql/mysql-server/mysql-5.1-bugteam
for merging into
https://code.launchpad.net/~mysql/mysql-server/mysql-5.1

2009/9/25 Tim Penhey <email address hidden>:
> I've manually grabbed the two branches you mention above, and I killed
> the merge --preview after it took over an hour and generated more than
> 33 meg of diff.

That would be worth filing as a performance bug against bzr. It may
be the diff is so large that it really does take ages but I think it
should be less than that.

--
Martin <http://launchpad.net/~mbp/>

Hi Tim, any news?

On Tue, 03 Nov 2009 22:38:01 GuilhemBichot wrote:
> Hi Tim, any news?

The actual underlying bug has been fixed for some time.

The bug to do with generating the huge diff slowly has been deployed, and
should be good now,

The bug around failing to parse the diff properly has also been handled, but
not fully fixed.

Should be good to go now.

I just created
https://code.launchpad.net/~mysql/mysql-server/mysql-pe/+merge/15418
and my subscription for branch lp:mysql-server (the target of the merge proposal) has "don't send diffs"... and I just received a 500kB diff attached to the email!?
So it still does not work.
FYI the mail had "To: <email address hidden>".

On Tue, 01 Dec 2009 02:31:41 GuilhemBichot wrote:
> I just created
> https://code.launchpad.net/~mysql/mysql-server/mysql-pe/+merge/15418
> and my subscription for branch lp:mysql-server (the target of the merge
> proposal) has "don't send diffs"... and I just received a 500kB diff
> attached to the email!? So it still does not work.
> FYI the mail had "To: <email address hidden>".

The solution isn't about "don't send diffs" though, it is about subscribing to
code reviews where only status updates are shown.

The don't send diffs is for the revision email.

We need to make that clearer, but I'm not using that field for this email.

Hi Tim. So is there a way that I get an email when a merge proposal is submitted, but without getting the diff?

On Tue, 01 Dec 2009 21:03:42 GuilhemBichot wrote:
> Hi Tim. So is there a way that I get an email when a merge proposal is
> submitted, but without getting the diff?

Yes. The third option when subscribing says:

Code review Level:
Control the kind of review activity that triggers notifications.

Status changes only - this will not send diffs.
Email about all changes - this will send diffs.

Oh man, just reading what I've written, it is not surprising that you can't
tell. Want to file a bug about that

Tim

Hello Tim. In my subscription I already have "code review level: status changes only" (and also: "notification level: branch attribute notifications only" and "generated diff size limit: don't send diffs"), but I got the diff yesterday with such settings: what should I change?
Or is there a bugfix I should wait for?

On Tue, 01 Dec 2009 21:41:34 GuilhemBichot wrote:
> Hello Tim. In my subscription I already have "code review level: status
> changes only" (and also: "notification level: branch attribute
> notifications only" and "generated diff size limit: don't send diffs"),
> but I got the diff yesterday with such settings: what should I change? Or
> is there a bugfix I should wait for?

Can you please forward me the email that you received from LP complete with
all the headers so I can check please.

On Tue, 01 Dec 2009 21:41:34 GuilhemBichot wrote:
> Hello Tim. In my subscription I already have "code review level: status
> changes only" (and also: "notification level: branch attribute
> notifications only" and "generated diff size limit: don't send diffs"),
> but I got the diff yesterday with such settings: what should I change? Or
> is there a bugfix I should wait for?

Ah... looking at the headers of the email that you forwarded to me, I can see
that this is a different special case.

If the registrant of the proposal is subscribed to get emails, we override the
subscription to send the registrant all emails, including comments, and the
diff. This is because we felt that the person proposing the merge is most
likely interested in following the conversation.

There is similar special case for the source branch owner, although only if
the owner of the source branch is not a team.

This could be changed, but I think that this is actually the expected
behaviour.

If you'd like me to propose the merge, you should see that you don't get the
email.

Hi Tim. Yes, please, propose the merge so that we verify that I don't get a diff. Thanks much!

Tim created a merge request and I received a notification without any diff. For me this bug is fixed.

@Guilhem, Aaron wrote up some description of how subscription works
here <https://help.launchpad.net/Code/Review/Relationships>. I
realize this bug is fixed for you but I thought you might still be
interested to read/comment on it.

--
Martin <http://launchpad.net/~mbp/>