Patchwork [STABLE] contrib: a small script to nudge lingering diff

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 24, 2020, 12:25 p.m.
Message ID <34b78bfc1afde8d03453.1579868721@nodosa.octobus.net>
Download mbox | patch
Permalink /patch/44605/
State New
Headers show

Comments

Pierre-Yves David - Jan. 24, 2020, 12:25 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1579866627 -3600
#      Fri Jan 24 12:50:27 2020 +0100
# Branch stable
# Node ID 34b78bfc1afde8d034538e23906ed2ce55ca169a
# Parent  ae596fac8ba072823ca9548b5360caa32a5d4840
# EXP-Topic phab-bot
# Available At https://dev.heptapod.net/octobus/mercurial-devel/
#              hg pull https://dev.heptapod.net/octobus/mercurial-devel/ -r 34b78bfc1afd
contrib: a small script to nudge lingering diff

After a discussion on IRC with various reviewers. It seems like a good idea to
have some automatic cleanup of old, inactive diffs.

Here is a small script able to do so. I am preparing to unleash it on our
phabricator instance.
Pierre-Yves David - Jan. 30, 2020, 6:05 p.m.
Gentle ping on that. Since this is actually put to use, I would rather 
have to in the soruce tree ASAP.

In addition there is a question of "where do we get it to run everyday". 
Right now I am running it by hand. I could set a recurrent job on an 
octobus server, however, I don't feel like it would be the right place 
for this.

On 1/24/20 1:25 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1579866627 -3600
> #      Fri Jan 24 12:50:27 2020 +0100
> # Branch stable
> # Node ID 34b78bfc1afde8d034538e23906ed2ce55ca169a
> # Parent  ae596fac8ba072823ca9548b5360caa32a5d4840
> # EXP-Topic phab-bot
> # Available At https://dev.heptapod.net/octobus/mercurial-devel/
> #              hg pull https://dev.heptapod.net/octobus/mercurial-devel/ -r 34b78bfc1afd
> contrib: a small script to nudge lingering diff
> 
> After a discussion on IRC with various reviewers. It seems like a good idea to
> have some automatic cleanup of old, inactive diffs.
> 
> Here is a small script able to do so. I am preparing to unleash it on our
> phabricator instance.
> 
> diff --git a/contrib/phab-clean.py b/contrib/phab-clean.py
> new file mode 100755
> --- /dev/null
> +++ b/contrib/phab-clean.py
> @@ -0,0 +1,93 @@
> +#!/usr/bin/env python
> +#
> +# A small script to automatically reject idle Diffs
> +#
> +# you need to set the PHABBOT_USER and PHABBOT_TOKEN environment variable for authentication
> +from __future__ import absolute_import, print_function
> +
> +import datetime
> +import os
> +import sys
> +
> +import phabricator
> +
> +MESSAGE = """There seems to have been no activities on this Diff for the past 3 Months.
> +
> +By policy, we are automatically moving it out of the `need-review` state.
> +
> +Please, move it back to `need-review` without hesitation if this diff should still be discussed.
> +
> +:baymax:need-review-idle:
> +"""
> +
> +
> +PHAB_URL = "https://phab.mercurial-scm.org/api/"
> +USER = os.environ.get("PHABBOT_USER", "baymax")
> +TOKEN = os.environ.get("PHABBOT_TOKEN")
> +
> +
> +NOW = datetime.datetime.now()
> +
> +# 3 months in seconds
> +DELAY = 60 * 60 * 24 * 30 * 3
> +
> +
> +def get_all_diff(phab):
> +    """Fetch all the diff that the need review"""
> +    return phab.differential.query(
> +        status="status-needs-review",
> +        order="order-modified",
> +        paths=[('HG', None)],
> +    )
> +
> +
> +def filter_diffs(diffs, older_than):
> +    """filter diffs to only keep the one unmodified sin <older_than> seconds"""
> +    olds = []
> +    for d in diffs:
> +        modified = int(d['dateModified'])
> +        modified = datetime.datetime.fromtimestamp(modified)
> +        d["idleFor"] = idle_for = NOW - modified
> +        if idle_for.total_seconds() > older_than:
> +            olds.append(d)
> +    return olds
> +
> +
> +def nudge_diff(phab, diff):
> +    """Comment on the idle diff and reject it"""
> +    diff_id = int(d['id'])
> +    phab.differential.createcomment(
> +        revision_id=diff_id, message=MESSAGE, action="reject"
> +    )
> +
> +
> +if not USER:
> +    print(
> +        "not user specified please set PHABBOT_USER and PHABBOT_TOKEN",
> +        file=sys.stderr,
> +    )
> +elif not TOKEN:
> +    print(
> +        "not api-token specified please set PHABBOT_USER and PHABBOT_TOKEN",
> +        file=sys.stderr,
> +    )
> +    sys.exit(1)
> +
> +phab = phabricator.Phabricator(USER, host=PHAB_URL, token=TOKEN)
> +phab.connect()
> +phab.update_interfaces()
> +print('Hello "%s".' % phab.user.whoami()['realName'])
> +
> +diffs = get_all_diff(phab)
> +print("Found %d Diffs" % len(diffs))
> +olds = filter_diffs(diffs, DELAY)
> +print("Found %d old Diffs" % len(olds))
> +for d in olds:
> +    diff_id = d['id']
> +    status = d['statusName']
> +    modified = int(d['dateModified'])
> +    idle_for = d["idleFor"]
> +    msg = 'nudging D%s in "%s" state for %s'
> +    print(msg % (diff_id, status, idle_for))
> +    # uncomment to actually affect phab
> +    nudge_diff(phab, d)
> diff --git a/tests/test-check-module-imports.t b/tests/test-check-module-imports.t
> --- a/tests/test-check-module-imports.t
> +++ b/tests/test-check-module-imports.t
> @@ -24,6 +24,7 @@ outputs, which should be fixed later.
>     > -X contrib/packaging/hg-docker \
>     > -X contrib/packaging/hgpackaging/ \
>     > -X contrib/packaging/inno/ \
> +  > -X contrib/phab-clean.py \
>     > -X contrib/python-zstandard/ \
>     > -X contrib/win32/hgwebdir_wsgi.py \
>     > -X contrib/perf-utils/perf-revlog-write-plot.py \
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Pulkit Goyal - Jan. 31, 2020, 4:10 p.m.
On Thu, Jan 30, 2020 at 9:15 PM Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
> Gentle ping on that. Since this is actually put to use, I would rather
> have to in the soruce tree ASAP.
>
> In addition there is a question of "where do we get it to run everyday".
> Right now I am running it by hand. I could set a recurrent job on an
> octobus server, however, I don't feel like it would be the right place
> for this.
>
> On 1/24/20 1:25 PM, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@octobus.net>
> > # Date 1579866627 -3600
> > #      Fri Jan 24 12:50:27 2020 +0100
> > # Branch stable
> > # Node ID 34b78bfc1afde8d034538e23906ed2ce55ca169a
> > # Parent  ae596fac8ba072823ca9548b5360caa32a5d4840
> > # EXP-Topic phab-bot
> > # Available At https://dev.heptapod.net/octobus/mercurial-devel/
> > #              hg pull https://dev.heptapod.net/octobus/mercurial-devel/ -r 34b78bfc1afd
> > contrib: a small script to nudge lingering diff
> >
> > After a discussion on IRC with various reviewers. It seems like a good idea to
> > have some automatic cleanup of old, inactive diffs.
> >
> > Here is a small script able to do so. I am preparing to unleash it on our
> > phabricator instance.
> >

Queuing this for default branch. Many many thanks!

> > diff --git a/contrib/phab-clean.py b/contrib/phab-clean.py
> > new file mode 100755
> > --- /dev/null
> > +++ b/contrib/phab-clean.py
> > @@ -0,0 +1,93 @@
> > +#!/usr/bin/env python
> > +#
> > +# A small script to automatically reject idle Diffs
> > +#
> > +# you need to set the PHABBOT_USER and PHABBOT_TOKEN environment variable for authentication
> > +from __future__ import absolute_import, print_function
> > +
> > +import datetime
> > +import os
> > +import sys
> > +
> > +import phabricator
> > +
> > +MESSAGE = """There seems to have been no activities on this Diff for the past 3 Months.
> > +
> > +By policy, we are automatically moving it out of the `need-review` state.
> > +
> > +Please, move it back to `need-review` without hesitation if this diff should still be discussed.
> > +
> > +:baymax:need-review-idle:
> > +"""
> > +
> > +
> > +PHAB_URL = "https://phab.mercurial-scm.org/api/"
> > +USER = os.environ.get("PHABBOT_USER", "baymax")
> > +TOKEN = os.environ.get("PHABBOT_TOKEN")
> > +
> > +
> > +NOW = datetime.datetime.now()
> > +
> > +# 3 months in seconds
> > +DELAY = 60 * 60 * 24 * 30 * 3
> > +
> > +
> > +def get_all_diff(phab):
> > +    """Fetch all the diff that the need review"""
> > +    return phab.differential.query(
> > +        status="status-needs-review",
> > +        order="order-modified",
> > +        paths=[('HG', None)],
> > +    )
> > +
> > +
> > +def filter_diffs(diffs, older_than):
> > +    """filter diffs to only keep the one unmodified sin <older_than> seconds"""
> > +    olds = []
> > +    for d in diffs:
> > +        modified = int(d['dateModified'])
> > +        modified = datetime.datetime.fromtimestamp(modified)
> > +        d["idleFor"] = idle_for = NOW - modified
> > +        if idle_for.total_seconds() > older_than:
> > +            olds.append(d)
> > +    return olds
> > +
> > +
> > +def nudge_diff(phab, diff):
> > +    """Comment on the idle diff and reject it"""
> > +    diff_id = int(d['id'])
> > +    phab.differential.createcomment(
> > +        revision_id=diff_id, message=MESSAGE, action="reject"
> > +    )
> > +
> > +
> > +if not USER:
> > +    print(
> > +        "not user specified please set PHABBOT_USER and PHABBOT_TOKEN",
> > +        file=sys.stderr,
> > +    )
> > +elif not TOKEN:
> > +    print(
> > +        "not api-token specified please set PHABBOT_USER and PHABBOT_TOKEN",
> > +        file=sys.stderr,
> > +    )
> > +    sys.exit(1)
> > +
> > +phab = phabricator.Phabricator(USER, host=PHAB_URL, token=TOKEN)
> > +phab.connect()
> > +phab.update_interfaces()
> > +print('Hello "%s".' % phab.user.whoami()['realName'])
> > +
> > +diffs = get_all_diff(phab)
> > +print("Found %d Diffs" % len(diffs))
> > +olds = filter_diffs(diffs, DELAY)
> > +print("Found %d old Diffs" % len(olds))
> > +for d in olds:
> > +    diff_id = d['id']
> > +    status = d['statusName']
> > +    modified = int(d['dateModified'])
> > +    idle_for = d["idleFor"]
> > +    msg = 'nudging D%s in "%s" state for %s'
> > +    print(msg % (diff_id, status, idle_for))
> > +    # uncomment to actually affect phab
> > +    nudge_diff(phab, d)
> > diff --git a/tests/test-check-module-imports.t b/tests/test-check-module-imports.t
> > --- a/tests/test-check-module-imports.t
> > +++ b/tests/test-check-module-imports.t
> > @@ -24,6 +24,7 @@ outputs, which should be fixed later.
> >     > -X contrib/packaging/hg-docker \
> >     > -X contrib/packaging/hgpackaging/ \
> >     > -X contrib/packaging/inno/ \
> > +  > -X contrib/phab-clean.py \
> >     > -X contrib/python-zstandard/ \
> >     > -X contrib/win32/hgwebdir_wsgi.py \
> >     > -X contrib/perf-utils/perf-revlog-write-plot.py \
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> >
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/contrib/phab-clean.py b/contrib/phab-clean.py
new file mode 100755
--- /dev/null
+++ b/contrib/phab-clean.py
@@ -0,0 +1,93 @@ 
+#!/usr/bin/env python
+#
+# A small script to automatically reject idle Diffs
+#
+# you need to set the PHABBOT_USER and PHABBOT_TOKEN environment variable for authentication
+from __future__ import absolute_import, print_function
+
+import datetime
+import os
+import sys
+
+import phabricator
+
+MESSAGE = """There seems to have been no activities on this Diff for the past 3 Months.
+
+By policy, we are automatically moving it out of the `need-review` state.
+
+Please, move it back to `need-review` without hesitation if this diff should still be discussed.
+
+:baymax:need-review-idle:
+"""
+
+
+PHAB_URL = "https://phab.mercurial-scm.org/api/"
+USER = os.environ.get("PHABBOT_USER", "baymax")
+TOKEN = os.environ.get("PHABBOT_TOKEN")
+
+
+NOW = datetime.datetime.now()
+
+# 3 months in seconds
+DELAY = 60 * 60 * 24 * 30 * 3
+
+
+def get_all_diff(phab):
+    """Fetch all the diff that the need review"""
+    return phab.differential.query(
+        status="status-needs-review",
+        order="order-modified",
+        paths=[('HG', None)],
+    )
+
+
+def filter_diffs(diffs, older_than):
+    """filter diffs to only keep the one unmodified sin <older_than> seconds"""
+    olds = []
+    for d in diffs:
+        modified = int(d['dateModified'])
+        modified = datetime.datetime.fromtimestamp(modified)
+        d["idleFor"] = idle_for = NOW - modified
+        if idle_for.total_seconds() > older_than:
+            olds.append(d)
+    return olds
+
+
+def nudge_diff(phab, diff):
+    """Comment on the idle diff and reject it"""
+    diff_id = int(d['id'])
+    phab.differential.createcomment(
+        revision_id=diff_id, message=MESSAGE, action="reject"
+    )
+
+
+if not USER:
+    print(
+        "not user specified please set PHABBOT_USER and PHABBOT_TOKEN",
+        file=sys.stderr,
+    )
+elif not TOKEN:
+    print(
+        "not api-token specified please set PHABBOT_USER and PHABBOT_TOKEN",
+        file=sys.stderr,
+    )
+    sys.exit(1)
+
+phab = phabricator.Phabricator(USER, host=PHAB_URL, token=TOKEN)
+phab.connect()
+phab.update_interfaces()
+print('Hello "%s".' % phab.user.whoami()['realName'])
+
+diffs = get_all_diff(phab)
+print("Found %d Diffs" % len(diffs))
+olds = filter_diffs(diffs, DELAY)
+print("Found %d old Diffs" % len(olds))
+for d in olds:
+    diff_id = d['id']
+    status = d['statusName']
+    modified = int(d['dateModified'])
+    idle_for = d["idleFor"]
+    msg = 'nudging D%s in "%s" state for %s'
+    print(msg % (diff_id, status, idle_for))
+    # uncomment to actually affect phab
+    nudge_diff(phab, d)
diff --git a/tests/test-check-module-imports.t b/tests/test-check-module-imports.t
--- a/tests/test-check-module-imports.t
+++ b/tests/test-check-module-imports.t
@@ -24,6 +24,7 @@  outputs, which should be fixed later.
   > -X contrib/packaging/hg-docker \
   > -X contrib/packaging/hgpackaging/ \
   > -X contrib/packaging/inno/ \
+  > -X contrib/phab-clean.py \
   > -X contrib/python-zstandard/ \
   > -X contrib/win32/hgwebdir_wsgi.py \
   > -X contrib/perf-utils/perf-revlog-write-plot.py \