[Twisted-Python] Spurious commit/ticket links
Hi, In the last couple days I've noticed that there are a bunch of spurious changes being made to tickets in the issue tracker. These come from commit messages that reference a GitHub PR that happens to match a ticket number in trac. For example, https://twistedmatrix.com/trac/ticket/600#comment:11 I guess this doesn't really hurt anything ... except it's dumping a constant low level of garbage into the issue tracker and generating some annoying emails (that end up having nothing to do with what the subject suggests). Jean-Paul
On Dec 1, 2016, at 10:51 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
Hi,
In the last couple days I've noticed that there are a bunch of spurious changes being made to tickets in the issue tracker. These come from commit messages that reference a GitHub PR that happens to match a ticket number in trac.
For example, https://twistedmatrix.com/trac/ticket/600#comment:11 <https://twistedmatrix.com/trac/ticket/600#comment:11>
I guess this doesn't really hurt anything ... except it's dumping a constant low level of garbage into the issue tracker and generating some annoying emails (that end up having nothing to do with what the subject suggests).
This is, unfortunately, going to keep happening more frequently as the PR numbers get higher and the corresponding Trac tickets get less sparse. The way I'd like to address it is to change the format of our commit message to namespace Trac tickets differently; instead of just "#", using a URL, like "Fixes https://tm.tl/1234". I wouldn't even mind if we just had to use the Trac wiki syntax for this, i.e. "Fixes [ticket:1234]" as long as we could turn off the "#" syntax which Github also uses. However, this involves surgery within Trac's code, and for me personally, the work required to find the relevant regexes and modify them is worse than continuing to deal with the annoyance. However, I would very much appreciate it if someone else would take this on :-). -glyph
On Thu, Dec 1, 2016 at 2:14 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 1, 2016, at 10:51 AM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
Hi,
In the last couple days I've noticed that there are a bunch of spurious changes being made to tickets in the issue tracker. These come from commit messages that reference a GitHub PR that happens to match a ticket number in trac.
For example, https://twistedmatrix.com/trac/ticket/600#comment:11
I guess this doesn't really hurt anything ... except it's dumping a constant low level of garbage into the issue tracker and generating some annoying emails (that end up having nothing to do with what the subject suggests).
This is, unfortunately, going to keep happening more frequently as the PR numbers get higher and the corresponding Trac tickets get less sparse.
The way I'd *like* to address it is to change the format of our commit message to namespace Trac tickets differently; instead of just "#", using a URL, like "Fixes https://tm.tl/1234". I wouldn't even mind if we just had to use the Trac wiki syntax for this, i.e. "Fixes [ticket:1234]" as long as we could turn off the "#" syntax which Github also uses.
However, this involves surgery within Trac's code, and for me personally, the work required to find the relevant regexes and modify them is worse than continuing to deal with the annoyance. However, I would very much appreciate it if someone else would take this on :-).
Where's the source for Twisted's trac deployment? Is it actually possible to deploy modifications? I'll take a look, if so. Jean-Paul
On Dec 1, 2016, at 12:13 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Thu, Dec 1, 2016 at 2:14 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Dec 1, 2016, at 10:51 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
Hi,
In the last couple days I've noticed that there are a bunch of spurious changes being made to tickets in the issue tracker. These come from commit messages that reference a GitHub PR that happens to match a ticket number in trac.
For example, https://twistedmatrix.com/trac/ticket/600#comment:11 <https://twistedmatrix.com/trac/ticket/600#comment:11>
I guess this doesn't really hurt anything ... except it's dumping a constant low level of garbage into the issue tracker and generating some annoying emails (that end up having nothing to do with what the subject suggests).
This is, unfortunately, going to keep happening more frequently as the PR numbers get higher and the corresponding Trac tickets get less sparse.
The way I'd like to address it is to change the format of our commit message to namespace Trac tickets differently; instead of just "#", using a URL, like "Fixes https://tm.tl/1234 <https://tm.tl/1234>". I wouldn't even mind if we just had to use the Trac wiki syntax for this, i.e. "Fixes [ticket:1234]" as long as we could turn off the "#" syntax which Github also uses.
However, this involves surgery within Trac's code, and for me personally, the work required to find the relevant regexes and modify them is worse than continuing to deal with the annoyance. However, I would very much appreciate it if someone else would take this on :-).
Where's the source for Twisted's trac deployment?
https://github.com/twisted-infra/twisted-trac-source https://github.com/twisted-infra/twisted-trac-plugins https://github.com/twisted-infra/braid/tree/master/services/trac
Is it actually possible to deploy modifications?
There's probably still an undocumented setup step or two that we've missed - but after following <https://github.com/twisted-infra/braid/blob/master/README.md> 'fab config.production trac.upgrade' ought to do the trick. Allegedly it's even possible to set up a test development environment as per <https://github.com/twisted-infra/braid/blob/master/README.md#vagrant> :-). I haven't made any major changes since all these docs were added so I'm just following them from the beginning for the first time myself now. but certainly the prod-deploy process has worked fine for me many times on various services.
I'll take a look, if so.
Please be vocal about any roadblocks you hit. The ops situation has improved a ton since the last time you looked, but (accordingly) it's also changed almost completely. Good luck - and hopefully you'll need a lot less of it than previously ;-). -glyph
On Thu, Dec 1, 2016 at 3:33 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Please be vocal about any roadblocks you hit. The ops situation has improved a ton since the last time you looked, but (accordingly) it's also changed almost completely.
Good luck - and hopefully you'll need a lot less of it than previously ;-).
Here's a new plugin that has the new behavior, maybe: https://github.com/twisted-infra/twisted-trac-plugins/pull/4 thijs did a review so maybe it's all set to merge. There's also a config change: https://github.com/twisted-infra/braid/pull/226 I had intended to test it with the local development environment but it looks like it requires downloading gigs of data for vagrant boxes and to upgrade a super-old ubuntu image. Maybe someone else could do that step. If not, maybe I'll test it on the production machine in a couple days. :) Jean-Paul
I deployed this to production. I don't see a good way to test it without screwing around with twisted trunk ... so don't plan to. I'll keep an eye on real merges folks are working on and see if it's behaving as desired. On Thu, Dec 1, 2016 at 8:50 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
On Thu, Dec 1, 2016 at 3:33 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Please be vocal about any roadblocks you hit. The ops situation has improved a ton since the last time you looked, but (accordingly) it's also changed almost completely.
Good luck - and hopefully you'll need a lot less of it than previously ;-).
Here's a new plugin that has the new behavior, maybe: https://github.com/twisted-infra/twisted-trac-plugins/pull/4 thijs did a review so maybe it's all set to merge. There's also a config change: https://github.com/ twisted-infra/braid/pull/226
I had intended to test it with the local development environment but it looks like it requires downloading gigs of data for vagrant boxes and to upgrade a super-old ubuntu image. Maybe someone else could do that step.
If not, maybe I'll test it on the production machine in a couple days. :)
Jean-Paul
On Dec 2, 2016, at 4:49 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
I deployed this to production. I don't see a good way to test it without screwing around with twisted trunk ... so don't plan to. I'll keep an eye on real merges folks are working on and see if it's behaving as desired.
Thanks, JP! Would you mind updating https://twistedmatrix.com/trac/wiki/ReviewProcess#Revertingachange and https://twistedmatrix.com/trac/wiki/ReviewProcess#Authors:Howtomergethechang... to reflect the new syntax that should be used on future merges? -glyph
On Fri, Dec 2, 2016 at 9:28 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 2, 2016, at 4:49 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
I deployed this to production. I don't see a good way to test it without screwing around with twisted trunk ... so don't plan to. I'll keep an eye on real merges folks are working on and see if it's behaving as desired.
Thanks, JP!
Would you mind updating https://twistedmatrix.com/trac/wiki/ReviewProcess# Revertingachange and https://twistedmatrix.com/trac/wiki/ReviewProcess# Authors:Howtomergethechangetotrunk to reflect the new syntax that should be used on future merges?
Updated. As far as the behavior, do https://github.com/twisted/twisted/pull/614 / http://twistedmatrix.com/trac/ticket/8934 look right? The trac ticket is fixed but with no comment. OTOH, I can't find the commit message for the PR merge so maybe that's as it should be?
-glyph
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On Sat, Dec 3, 2016 at 9:28 AM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
On Fri, Dec 2, 2016 at 9:28 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 2, 2016, at 4:49 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
I deployed this to production. I don't see a good way to test it without screwing around with twisted trunk ... so don't plan to. I'll keep an eye on real merges folks are working on and see if it's behaving as desired.
Thanks, JP!
Would you mind updating https://twistedmatrix.com/trac /wiki/ReviewProcess#Revertingachange and https://twistedmatrix.com/trac /wiki/ReviewProcess#Authors:Howtomergethechangetotrunk to reflect the new syntax that should be used on future merges?
Updated. As far as the behavior, do https://github.com/twisted/ twisted/pull/614 / http://twistedmatrix.com/trac/ticket/8934 look right? The trac ticket is fixed but with no comment. OTOH, I can't find the commit message for the PR merge so maybe that's as it should be?
Updated again. Craig confirmed that it indeed wasn't working (at all). Also, the site's now running trac 1.2 (was 1.0.12). The fab target for re-deploying trac just picks the newest release at the moment, despite the *appearance* of being pinned to a particular version. I don't think I'm going to try to bite off fixing this. The problem, though, is multiple invocations of `pip -U` - a later instance of which is totally happy to upgrade the trac 1.0.12 install to 1.2. Jean-Paul
-glyph
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On Dec 5, 2016, at 2:45 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Sat, Dec 3, 2016 at 9:28 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote: On Fri, Dec 2, 2016 at 9:28 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Dec 2, 2016, at 4:49 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
I deployed this to production. I don't see a good way to test it without screwing around with twisted trunk ... so don't plan to. I'll keep an eye on real merges folks are working on and see if it's behaving as desired.
Thanks, JP!
Would you mind updating https://twistedmatrix.com/trac/wiki/ReviewProcess#Revertingachange <https://twistedmatrix.com/trac/wiki/ReviewProcess#Revertingachange> and https://twistedmatrix.com/trac/wiki/ReviewProcess#Authors:Howtomergethechang... <https://twistedmatrix.com/trac/wiki/ReviewProcess#Authors:Howtomergethechang...> to reflect the new syntax that should be used on future merges?
Updated. As far as the behavior, do https://github.com/twisted/twisted/pull/614 <https://github.com/twisted/twisted/pull/614> / http://twistedmatrix.com/trac/ticket/8934 <http://twistedmatrix.com/trac/ticket/8934> look right? The trac ticket is fixed but with no comment. OTOH, I can't find the commit message for the PR merge so maybe that's as it should be?
Updated again. Craig confirmed that it indeed wasn't working (at all).
Also, the site's now running trac 1.2 (was 1.0.12). The fab target for re-deploying trac just picks the newest release at the moment, despite the appearance of being pinned to a particular version. I don't think I'm going to try to bite off fixing this. The problem, though, is multiple invocations of `pip -U` - a later instance of which is totally happy to upgrade the trac 1.0.12 install to 1.2.
Now I can't create tickets at all. I hope this is related and not just general decay of our Trac instance :). The web UI tells me "Warning: The action "None" is not available" without actually creating the ticket. Nothing that I can see appears in the logs. Hopefully this warning message means something to someone? -glyph
On Tue, Dec 6, 2016 at 1:08 AM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Now I can't create tickets at all. I hope this is related and not just general decay of our Trac instance :).
The web UI tells me "Warning: The action "None" is not available" without actually creating the ticket. Nothing that I can see appears in the logs. Hopefully this warning message means something to someone?
Not to me, unfortunately. :(
-glyph
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On Dec 6, 2016, at 5:04 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Tue, Dec 6, 2016 at 1:08 AM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
Now I can't create tickets at all. I hope this is related and not just general decay of our Trac instance :).
The web UI tells me "Warning: The action "None" is not available" without actually creating the ticket. Nothing that I can see appears in the logs. Hopefully this warning message means something to someone?
Not to me, unfortunately. :(
Luckily it meant something to Amber! -glyph
On Dec 5, 2016, at 2:45 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
Updated again. Craig confirmed that it indeed wasn't working (at all).
Also, the site's now running trac 1.2 (was 1.0.12). The fab target for re-deploying trac just picks the newest release at the moment, despite the appearance of being pinned to a particular version. I don't think I'm going to try to bite off fixing this. The problem, though, is multiple invocations of `pip -U` - a later instance of which is totally happy to upgrade the trac 1.0.12 install to 1.2.
Another problem commit: https://github.com/twisted/twisted/commit/6ec4521caae4edec97238ae00ff9bf81a4... This should have reopened https://twistedmatrix.com/trac/ticket/8937 but didn't. Have there been any examples of it working yet? -glyph
On Thu, Dec 8, 2016 at 8:49 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 5, 2016, at 2:45 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
Updated again. Craig confirmed that it indeed wasn't working (at all).
Also, the site's now running trac 1.2 (was 1.0.12). The fab target for re-deploying trac just picks the newest release at the moment, despite the *appearance* of being pinned to a particular version. I don't think I'm going to try to bite off fixing this. The problem, though, is multiple invocations of `pip -U` - a later instance of which is totally happy to upgrade the trac 1.0.12 install to 1.2.
Another problem commit:
https://github.com/twisted/twisted/commit/6ec4521caae4edec97238ae00ff9bf 81a4701a73
This should have reopened https://twistedmatrix.com/trac/ticket/8937 but didn't.
Have there been any examples of it working yet?
(Followed-up on IRC in Twisted admin channe; glyph can summarize if he
wants. -glyph
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On Dec 8, 2016, at 7:03 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Thu, Dec 8, 2016 at 8:49 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Dec 5, 2016, at 2:45 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
Updated again. Craig confirmed that it indeed wasn't working (at all).
Also, the site's now running trac 1.2 (was 1.0.12). The fab target for re-deploying trac just picks the newest release at the moment, despite the appearance of being pinned to a particular version. I don't think I'm going to try to bite off fixing this. The problem, though, is multiple invocations of `pip -U` - a later instance of which is totally happy to upgrade the trac 1.0.12 install to 1.2.
Another problem commit:
https://github.com/twisted/twisted/commit/6ec4521caae4edec97238ae00ff9bf81a4... <https://github.com/twisted/twisted/commit/6ec4521caae4edec97238ae00ff9bf81a4...>
This should have reopened https://twistedmatrix.com/trac/ticket/8937 <https://twistedmatrix.com/trac/ticket/8937> but didn't.
Have there been any examples of it working yet?
(Followed-up on IRC in Twisted admin channe; glyph can summarize if he wants.
The part that I remember was just fixing trac after it broke - but hopefully what broke it was an update that fixes the regex :). Hopefully I'll close another ticket tonight and test it out... -g
On Dec 8, 2016, at 10:54 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 8, 2016, at 7:03 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
On Thu, Dec 8, 2016 at 8:49 PM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Dec 5, 2016, at 2:45 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
Updated again. Craig confirmed that it indeed wasn't working (at all).
Also, the site's now running trac 1.2 (was 1.0.12). The fab target for re-deploying trac just picks the newest release at the moment, despite the appearance of being pinned to a particular version. I don't think I'm going to try to bite off fixing this. The problem, though, is multiple invocations of `pip -U` - a later instance of which is totally happy to upgrade the trac 1.0.12 install to 1.2.
Another problem commit:
https://github.com/twisted/twisted/commit/6ec4521caae4edec97238ae00ff9bf81a4... <https://github.com/twisted/twisted/commit/6ec4521caae4edec97238ae00ff9bf81a4...>
This should have reopened https://twistedmatrix.com/trac/ticket/8937 <https://twistedmatrix.com/trac/ticket/8937> but didn't.
Have there been any examples of it working yet?
(Followed-up on IRC in Twisted admin channe; glyph can summarize if he wants.
The part that I remember was just fixing trac after it broke - but hopefully what broke it was an update that fixes the regex :). Hopefully I'll close another ticket tonight and test it out...
I did test it out, aaaaand: https://twistedmatrix.com/trac/ticket/8922#comment:5 It works! \o/ -glyph
On Thu, Dec 1, 2016 at 3:13 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
On Thu, Dec 1, 2016 at 2:14 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 1, 2016, at 10:51 AM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
Hi,
In the last couple days I've noticed that there are a bunch of spurious changes being made to tickets in the issue tracker. These come from commit messages that reference a GitHub PR that happens to match a ticket number in trac.
For example, https://twistedmatrix.com/trac/ticket/600#comment:11
I guess this doesn't really hurt anything ... except it's dumping a constant low level of garbage into the issue tracker and generating some annoying emails (that end up having nothing to do with what the subject suggests).
This is, unfortunately, going to keep happening more frequently as the PR numbers get higher and the corresponding Trac tickets get less sparse.
The way I'd *like* to address it is to change the format of our commit message to namespace Trac tickets differently; instead of just "#", using a URL, like "Fixes https://tm.tl/1234". I wouldn't even mind if we just had to use the Trac wiki syntax for this, i.e. "Fixes [ticket:1234]" as long as we could turn off the "#" syntax which Github also uses.
However, this involves surgery within Trac's code, and for me personally, the work required to find the relevant regexes and modify them is worse than continuing to deal with the annoyance. However, I would very much appreciate it if someone else would take this on :-).
Where's the source for Twisted's trac deployment? Is it actually possible to deploy modifications? I'll take a look, if so.
https://trac.edgewall.org/browser/trunk/tracopt/ticket/commit_updater.py#L14... looks like the relevant regexp. Removing the "#" branch would probably produce the desired behavior. However, also see https://trac.edgewall.org/wiki/CommitTicketUpdater#Configuration. I wonder if commit_ticket_update_envelope would also fix it? I don't think so because I think these are just "references" not "commands", but I don't know for sure. Jean-Paul
Jean-Paul
participants (2)
-
Glyph Lefkowitz -
Jean-Paul Calderone