• 
      

    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. 15, 2009 and submitted

    Information

    RBTools

    Reviewers

    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
    
        -- 
        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:
    
    
            Hi,
    
            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']. ['10.217.1.4']).
            Subsequent calls to gethostbyaddr roundrobin the list
            ...('perforce01', ['perforce04', 'perforce']. ['10.217.1.4'])
            ...('perforce', ['perforce01', 'perforce04']. ['10.217.1.4'])
            ...('perforce04', ['perforce', 'perforce01']. ['10.217.1.4'])
    
            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?
    
            thanks,
            Ravi.
    
     
    chipx86
    1. 
        
    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.
    11. 
        
    AR
    AR
    Review request changed
    Change Summary:
    1. removed a trailing space.
    chipx86
    1. Thanks. Committed with minor style modifications as ra39e3f2.
    2.