1045736 - Discover review repo automatically

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P2)

Description

[Gregory Szorc [:gps]]

I think it would be rad if the reviewboard client Mercurial extension discovered the path to the appropriate review repository automagically.

We can map local repositories to a review repository by comparing the SHA-1 of the initial commit (rev 0). If they are identical, that's your review repo. aurora, beta, release, etc would all map into the "gecko" repo because they all share the same root commit.

We can do this by having the server expose a map of paths to root commit SHA-1. Here's what I'm thinking.

1) CRON or RB mechanism on server that iterates known review repos and captures their root commit and writes it to a file.
2) RB server hg extension exposes this map through the wire protocol.
3) `hg push review` will fetch this map (if necessary) and adjust the remote URL automagically.

We can hard-code `review` as a [path] in the client RB extension.

Comment 1

[Gregory Szorc [:gps]]

I keep saying I'm going to do this and I have every intention of making it happen. I love user interfaces that "just work."

Comment 2

[Gregory Szorc [:gps]]

Also, the initial comment should be disregarded. A wire protocol request to the Mercurial server can issue a RBClient API call to discover the repositories. We can then iterate and discover rev 0 and return that mapping. We may need to throw in a layer of caching as the number of repos increases to make it scale.

Comment 3

[Gregory Szorc [:gps]]

This is a feature enhancement, that a P1 bug.

Comment 4

[Gregory Szorc [:gps]]

Attachment: MozReview Request: bz://1045736/gps

Comment 5

[Gregory Szorc [:gps]]

/r/47 - reviewboard: add a make-admin command
/r/49 - reviewboard: add a delete-repo command
/r/51 - reviewboard: implement automatic repository discovery (bug 1045736)

Pull down these commits:

hg pull review -r 9c19776b26cd43667466f92e7a6bf529c882cfc8

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8513813 [details]
MozReview Request: bz://1045736/gps

/r/51 - reviewboard: implement automatic repository discovery (bug 1045736)

Pull down this commit:

hg pull review -r ffb1b2a5d115bdbc870e99d0e730db7bf7259693

Comment 7

[Monty the Python Review Bot]

https://reviewboard.mozilla.org/r/51/#review2305

Always look on the bright side of life.

I analyzed your Python changes and found 7 errors.

The following files were examined:

  hgext/reviewboard/client.py
  hgext/reviewboard/server.py

::: hgext/reviewboard/client.py
(Diff revision 2)
> +            return orig(repo, remote, force=force, revs=revs,
> +                    newbranch=newbranch, **kwargs)

E128: continuation line under-indented for visual indent
Continuation lines indentation.

Continuation lines should align wrapped elements either vertically
using Python's implicit line joining inside parentheses, brackets
and braces, or using a hanging indent.

When using a hanging indent these considerations should be applied:
- there should be no arguments on the first line, and
- further indentation should be used to clearly distinguish itself as a
continuation line.

::: hgext/reviewboard/client.py
(Diff revision 2)
> +            raise util.Abort(_('refusing to redirect due to URL mismatch: %s' %
> +                newurl))

E128: continuation line under-indented for visual indent
Continuation lines indentation.

Continuation lines should align wrapped elements either vertically
using Python's implicit line joining inside parentheses, brackets
and braces, or using a hanging indent.

When using a hanging indent these considerations should be applied:
- there should be no arguments on the first line, and
- further indentation should be used to clearly distinguish itself as a
continuation line.

::: hgext/reviewboard/client.py
(Diff revision 2)
> +            raise util.Abort(_('do not know how to talk to this remote type\n'))

E501: line too long (80 > 79 characters)
Limit all lines to a maximum of 79 characters.

There are still many devices around that are limited to 80 character
lines; plus, limiting windows to 80 characters makes it possible to have
several windows side-by-side.  The default wrapping on such devices looks
ugly.  Therefore, please limit all lines to a maximum of 79 characters.
For flowing long blocks of text (docstrings or comments), limiting the
length to 72 characters is recommended.

Reports error E501.

::: hgext/reviewboard/server.py
(Diff revision 2)
>          _('REVIEWBOARD: See https://hg.mozilla.org/hgcustom/version-control-tools/file/tip/hgext/reviewboard/README.rst\n'))
>  
> +def listreviewrepos(repo):

E302: expected 2 blank lines, found 1
Separate top-level function and class definitions with two blank lines.

Method definitions inside a class are separated by a single blank line.

Extra blank lines may be used (sparingly) to separate groups of related
functions.  Blank lines may be omitted between a bunch of related
one-liners (e.g. a set of dummy implementations).

Use blank lines in functions, sparingly, to indicate logical sections.

::: hgext/reviewboard/server.py
(Diff revision 2)
> +    from rbtools.api.errors import APIError

F401: 'APIError' imported but unused

::: hgext/reviewboard/server.py
(Diff revision 2)
> +    #repo.vfs.writelines('reviewrepos', lines)

E265: block comment should start with '# '
Separate inline comments by at least two spaces.

An inline comment is a comment on the same line as a statement.  Inline
comments should be separated by at least two spaces from the statement.
They should start with a # and a single space.

Each line of a block comment starts with a # and a single space
(unless it is indented text inside the comment).

::: hgext/reviewboard/server.py
(Diff revision 2)
> -    if not ui.configint('reviewboard', 'repoid', None):
> +    if not ui.configint('reviewboard', 'repoid', None) and not ui.configbool('reviewboard', 'discoveryrepo', False):

E501: line too long (116 > 79 characters)
Limit all lines to a maximum of 79 characters.

There are still many devices around that are limited to 80 character
lines; plus, limiting windows to 80 characters makes it possible to have
several windows side-by-side.  The default wrapping on such devices looks
ugly.  Therefore, please limit all lines to a maximum of 79 characters.
For flowing long blocks of text (docstrings or comments), limiting the
length to 72 characters is recommended.

Reports error E501.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8513813 [details]
MozReview Request: bz://1045736/gps

/r/51 - reviewboard: implement automatic repository discovery (bug 1045736)

Pull down this commit:

hg pull -r 9b9ee25bea739646529bceb40d9070ad5c04802a https://reviewboard-hg.mozilla.org/version-control-tools/

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8513813 [details]
MozReview Request: bz://1045736/gps

/r/51 - reviewboard: implement automatic repository discovery (bug 1045736)
/r/7903 - docs: update documentation to reflect auto-review repository

Pull down these commits:

hg pull -r fd79293155b4bd0d5e5faef5b205ae956ab2dd12 https://reviewboard-hg.mozilla.org/version-control-tools/

Comment 10

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/51/#review6727

Looks great, thanks for taking another look at this!

It'd be great if you could go through Monty's old issues and make sure they're fixed or irrelevant and close them out in the review.

::: hgext/reviewboard/server.py:152
(Diff revision 3)
> +    repos = root.get_repositories(max_results=1000, tool='Mercurial')

FYI: Review Board will never return more than 250 results per page, even if you request it (so you'll at most get 250 per page here). RB has this as a hardcoded limit.

::: hgext/reviewboard/tests/test-repo-discovery.t:97
(Diff revision 3)
> +Pushing an repository unrelated to any known repo should result in error message

"an" -> "a"

::: hgext/reviewboard/server.py:156
(Diff revision 3)
> +                urls.add(r.path)

I wonder if we should start populating "mirror path" on the review board end, and start handing that our instead of the path review board uses itself. That way we could have RB talk to a different url.

Really maybe we should just have a MozReview hosting service where we can populate metadata about repos etc that meets our needs.

We don't need either of above right now, just thoughts for paths to take in the future.

Comment 11

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/7903/#review6733

Ship It!

Comment 12

[Steven MacLeod [:smacleod]]

Comment on attachment 8513813 [details]
MozReview Request: bz://1045736/gps

https://reviewboard.mozilla.org/r/45/#review6735

Ship It!

Comment 13

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/0b02cd388590

Comment 14

[Gregory Szorc [:gps]]

I deployed this code to reviewboard-hg.mozilla.org and created an autoreview repo. It "just works."

Comment 16

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: add a delete-repo command

Comment 17

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: implement automatic repository discovery (bug 1045736)

Comment 18

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: add a make-admin command

Comment 19

[Gregory Szorc [:gps]]

Attachment: MozReview Request: docs: update documentation to reflect auto-review repository