Handles a case where perforce server name has aliases. Postreview selects the alias that is registered on reviewboard and uses it as repository_path when posting new review request

Review Request #1235 — Created Nov. 14, 2009 and submitted




The patch provides changes to implement option (2) listed in the email below from Christian Hammond. The problem being fixed is described in the original email from Ravi.

On Sat, Nov 14, 2009 at 4:48 PM, Christian Hammond <chipx86@chipx86.com> wrote:

    Hi Ravi,

    There's no good solution to this today. Right now, it needs one perforce repository it can rely upon, and if you were to add multiple repositories, it would not work. In the future I really would like to make our repository matching more flexible, but it's not trivial.

    There are two things I can think of doing:

    1) Short-term, edit your hosts file to map perforce4 to the IP. It *should* take precedence instead of looking it up round-robin. However, this may affect other things negatively, and everyone would have to do it.

    2) Rework postreview.py to support lists of equivalent repository paths.

    Right now, each SCMClient in postreview.py generates a RepositoryInfo object containing the repository path. The path is a string, and in this case is returned from gethostbyaddr(hostname)[0]. This could be changed to optionally support a list instead of a string. In the case of a list, we'd check each item for a match on the server.

    This would require re-working the ReviewBoardServer.new_review_request function. Right now it just takes the path given and feeds it in to the call. What we would have to do instead is make a call to get the list of all repositories (you can call get_repositories() for that). We'd do this only if the paths were a list instead of a string. We'd look through the list returned by the server, see if we can find any of the ones we've found and, if we do find one, we'd use that value for new_review_request.

    I don't have a good setup here to test with. Would you be willing to give this a shot?


    Christian Hammond - chipx86@chipx86.com
    Review Board - http://www.reviewboard.org
    VMware, Inc. - http://www.vmware.com

    On Fri, Nov 13, 2009 at 3:44 PM, RaviKondamuru <ravikondamuru@gmail.com> wrote:


        When running post-review to upload a perforce changelist to
        reviewboard, I often encounter the error: "Error creating review
        request: The repository path specified is not in the list of known
        repositories (code 206)". On looking to post-review, I found the issue
        to be in the code that determines repository_path:
                  info = socket.gethostbyaddr(hostname)
                  repository_path = "%s:%s" % (info[0], port)

        while the hostname is 'perforce04',
        info has the following value:
        ...('perforce04', ['perforce', 'perforce01']. ['']).
        Subsequent calls to gethostbyaddr roundrobin the list
        ...('perforce01', ['perforce04', 'perforce']. [''])
        ...('perforce', ['perforce01', 'perforce04']. [''])
        ...('perforce04', ['perforce', 'perforce01']. [''])

        So each time the repository changes. And since I have set the
        repository on reviewboard to be perforce04, every time the
        repository_path is perforce or perforce01, it gives the above error.

        what is the significance of repostitory in reviewboard?
        Can I add all 3 of perforce server aliases in reviewboard as
        respositories? if yes, what happens if subsequent upload of diffs for
        the same changelist result in different respository_path. Will that
        cause an issue?


  2. rbtools/postreview.py (Diff revision 1)
    We need to support Python 2.4. This is too new.
  3. rbtools/postreview.py (Diff revision 1)
    Trailing whitespace.
  4. rbtools/postreview.py (Diff revision 1)
    Can do: if isinstance(self.info.path, list):
  5. rbtools/postreview.py (Diff revision 1)
    Blank line between these.
  6. rbtools/postreview.py (Diff revision 1)
    Trailing whitespace.
    Make sure that the parameters all align.
  7. rbtools/postreview.py (Diff revision 1)
    Blank line between these.
  8. rbtools/postreview.py (Diff revision 1)
    Can probably just do:
    if info[1]:
  9. rbtools/postreview.py (Diff revision 1)
    Can simplify to:
    servers = [info[0]] + info[1]
  10. rbtools/postreview.py (Diff revision 1)
    Space on both sides of %.
    Make sure the whole thing fits in < 80 chars.
Review request changed
Change Summary:
1. removed a trailing space.
  1. Thanks. Committed with minor style modifications as ra39e3f2.