• 
      

    Fix some validation issues and error display on Bitbucket.

    Review Request #6359 — Created Sept. 23, 2014 and submitted

    Information

    Review Board
    release-2.0.x
    a8bf629...

    Reviewers

    It was common for people to put invalid repository names or usernames
    into the Bitbucket repository config form. When this happened, we'd
    either end up storing bad account information, fail with a cryptic
    error, or crash.

    Some of the most common problems here have been fixed.

    We now check the the validity of the username and password prior to
    storage. We fetch the user session API, using the credentials provided
    by the user. The credential data is only stored if this succeeds.

    If any API request results in an HTTP 401, we raise an
    AuthorizationError. All other errors result in a HostingServiceError
    (previously, we'd raise an Exception, which wouldn't always be caught
    correctly). If the data from the server was a valid error payload, we
    now parse that and display the error message.

    Repositories are now validated properly as well. A repository name that
    looks like a path (has a '/') or contains '.git' will immediately result
    in an error instructing the user to provide just the name. If a name
    meets the criteria, but is not found, a suitable error will also be
    shown instead of a JSON payload.

    Tested each failure condition manually.

    Tested successful authentication and repository access.

    Unit tests pass.

    Description From Last Updated

    local variable 'data' is assigned to but never used

    reviewbot reviewbot

    local variable 'e' is assigned to but never used

    reviewbot reviewbot

    'HostingServiceError' imported but unused

    reviewbot reviewbot

    undefined name 'HostingServiceError'

    reviewbot reviewbot

    undefined name 'HostingServiceError'

    reviewbot reviewbot

    'HostingServiceError' imported but unused

    reviewbot reviewbot

    Should these be using gettext or ugettext?

    david david

    This should be six.text_type(message) or ...

    david david
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
    2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 1)
       
       
      Show all issues
       local variable 'data' is assigned to but never used
      
    3. reviewboard/hostingsvcs/bitbucket.py (Diff revision 1)
       
       
      Show all issues
       local variable 'e' is assigned to but never used
      
    4. reviewboard/hostingsvcs/tests.py (Diff revision 1)
       
       
      Show all issues
       'HostingServiceError' imported but unused
      
    5. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
    2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues
       undefined name 'HostingServiceError'
      
    3. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues
       undefined name 'HostingServiceError'
      
    4. reviewboard/hostingsvcs/tests.py (Diff revision 2)
       
       
      Show all issues
       'HostingServiceError' imported but unused
      
    5. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 3)
       
       
      Show all issues

      Should these be using gettext or ugettext?

      1. Probably ugettext. Fixing.

    3. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 4)
       
       
      Show all issues

      This should be six.text_type(message) or ...

    3. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/bitbucket.py
      
      
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.0.x (6cbe372)