• 
      

    Sandbox reviewboard.hostingsvcs.service.HostingService

    Review Request #6652 — Created Nov. 28, 2014 and discarded

    Information

    Review Board
    master

    Reviewers

    Extensions that create a HostingService subclass (using the HostingServiceHook) can throw exceptions inside Reviewboard. The parts of Reviewboard that call those methods have been sandboxed.

    Now when a HostingService subclass from an extension throws an exception; Reviewboard logs the errors with enough information to find which method in the HostingService subclass threw the exception.

    Unit tests have been written to make sure that functions from a Widget subclass have been called, and when an exception is thrown it gets logged.

    The tests fail without the sanboxing, and succeed with it.

    Description From Last Updated

    'SCMTool' imported but unused

    reviewbotreviewbot

    This will make it fail silently rather than reporting any errors to the user.

    daviddavid

    This is still making it fail silently (reporting zero branches). Before it would tell the user about an error occurring. …

    daviddavid

    Same here.

    daviddavid

    And here.

    daviddavid
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/hostingsvcs/tests.py
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/hostingsvcs/tests.py
      
      
    2. 
        
    justy777
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/scmtools/tests.py
          reviewboard/hostingsvcs/models.py
          reviewboard/hostingsvcs/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/scmtools/tests.py
          reviewboard/hostingsvcs/models.py
          reviewboard/hostingsvcs/tests.py
      
      
    2. reviewboard/scmtools/tests.py (Diff revision 2)
       
       
      Show all issues
       'SCMTool' imported but unused
      
    3. 
        
    justy777
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/scmtools/tests.py
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/service.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/scmtools/tests.py
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/hostingsvcs/tests.py
          reviewboard/hostingsvcs/service.py
      
      
    2. 
        
    justy777
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/webapi/resources/remote_repository.py
          reviewboard/scmtools/tests.py
          reviewboard/webapi/tests/test_remote_repository.py
          reviewboard/hostingsvcs/service.py
          reviewboard/hostingsvcs/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/webapi/resources/remote_repository.py
          reviewboard/scmtools/tests.py
          reviewboard/webapi/tests/test_remote_repository.py
          reviewboard/hostingsvcs/service.py
          reviewboard/hostingsvcs/tests.py
      
      
    2. 
        
    justy777
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/webapi/resources/remote_repository.py
          reviewboard/scmtools/tests.py
          reviewboard/scmtools/admin.py
          reviewboard/webapi/tests/test_remote_repository.py
          reviewboard/hostingsvcs/service.py
          reviewboard/hostingsvcs/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/webapi/resources/remote_repository.py
          reviewboard/scmtools/tests.py
          reviewboard/scmtools/admin.py
          reviewboard/webapi/tests/test_remote_repository.py
          reviewboard/hostingsvcs/service.py
          reviewboard/hostingsvcs/tests.py
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/scmtools/models.py (Diff revision 5)
       
       
       
       
      Show all issues

      This will make it fail silently rather than reporting any errors to the user.

    3. reviewboard/scmtools/models.py (Diff revision 5)
       
       
       
       
      Show all issues

      Same here.

    4. reviewboard/scmtools/models.py (Diff revision 5)
       
       
       
      Show all issues

      And here.

    5. 
        
    justy777
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/webapi/resources/remote_repository.py
          reviewboard/scmtools/tests.py
          reviewboard/scmtools/admin.py
          reviewboard/webapi/tests/test_remote_repository.py
          reviewboard/hostingsvcs/service.py
          reviewboard/hostingsvcs/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/models.py
          reviewboard/scmtools/models.py
          reviewboard/webapi/resources/remote_repository.py
          reviewboard/scmtools/tests.py
          reviewboard/scmtools/admin.py
          reviewboard/webapi/tests/test_remote_repository.py
          reviewboard/hostingsvcs/service.py
          reviewboard/hostingsvcs/tests.py
      
      
    2. 
        
    david
    1. In your testing done, you say "Unit tests have been written to make sure that functions from a Widget subclass have been called, and when an exception is thrown it gets logged." That doesn't seem correct for this change.

    2. reviewboard/scmtools/models.py (Diff revisions 5 - 6)
       
       
      Show all issues

      This is still making it fail silently (reporting zero branches). Before it would tell the user about an error occurring.

      These methods here should probably not be wrapped at all, because we expect them to throw exceptions and the proper error handling is already in place.

    3. 
        
    justy777
    Review request changed
    Status:
    Discarded