Webapi Resource for integrations

Review Request #7105 - Created March 23, 2015 and discarded

Xuanyi Lin
Review Board
master
6918
7221, 7136, 7106
reviewboard, students

This provides the webapi resource and the views for the integration admin page.

The webapi resource for both Integration and ConfiguredIntegration are provided here. The Integration resource allows for getting the list of Integrations that implements the class and hook defined in review 6918.

The ConfiguredIntegration resource allows GET, PUT and DELETE call to manage individual configured integration.

Unit test done.
Tested with WebapiResource test for both integrations and configuredIntegration resource.

  • 12
  • 0
  • 23
  • 2
  • 37
Description From Last Updated
No blank line here. David Trowbridge David Trowbridge
All str in here should be six.text_type for future Python 3 compatibility. Here and below. David Trowbridge David Trowbridge
I think I'd prefer if these conditionals were switched: if config and config.integration: return config.integration.name else: return None David Trowbridge David Trowbridge
Same as above. David Trowbridge David Trowbridge
Same as above. David Trowbridge David Trowbridge
Same as above. David Trowbridge David Trowbridge
This should have a docstring. David Trowbridge David Trowbridge
This should have a docstring. David Trowbridge David Trowbridge
No blank line here. David Trowbridge David Trowbridge
All str items in here should be six.text_type David Trowbridge David Trowbridge
Please switch the conditional to test if integration: and return None in the else clause. David Trowbridge David Trowbridge
This should have a docstring. David Trowbridge David Trowbridge
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/integrations/admin.py
    
    
  2. 
      
Xuanyi Lin
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
  2.  'NOT_LOGGED_IN' imported but unused
    
  3.  'PERMISSION_DENIED' imported but unused
    
  4.  'webapi_response_errors' imported but unused
    
  5.  'webapi_login_required' imported but unused
    
  6.  'webapi_check_local_site' imported but unused
    
  7. 
      
Xuanyi Lin
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
  2. 
      
Xuanyi Lin
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
  2.  local variable 'manager' is assigned to but never used
    
  3. 
      
Xuanyi Lin
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
  2.  'IntegrationManager' imported but unused
    
  3. 
      
Xuanyi Lin
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
  2. 
      
Barret Rennie
  1. Can you change references to "webapi" in your summary and description to "WebAPI" ?

  2. reviewboard/integrations/views.py (Diff revision 6)
     
     

    This function needs a docstring.

  3. reviewboard/integrations/views.py (Diff revision 6)
     
     

    This function needs a docstring.

  4. reviewboard/webapi/resources/__init__.py (Diff revision 6)
     
     
     

    Please alphabetize these.

  5. Undo this change.

  6. This method isn't called from anywhere.

    1. I understand how this works now, dropping.

  7. 
      
Xuanyi Lin
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
  2. 
      
Barret Rennie
  1. 
      
  2. reviewboard/integrations/views.py (Diff revision 7)
     
     
     
     
     
     
     
     

    This will raise an exception if integration_class is None. You should define integration as None before the if integration_class block.

  3. ConfiguredIntegration.

  4. reviewboard/webapi/resources/configured_integration.py (Diff revision 7)
     
     
     
     
     
     
     

    This docstring will end up in public facing documentation. It should not mention class names of RB-specific things (e.g., ConfiguredIntegration). Instead it should just refer to, e.g., "configuration integration".

  5. ConfiguredIntegration

  6. When is_list is false, this will return a list. Shouldn't it return a single instance?

    1. The superclass's get_object method in WebAPI resource will be calling get_queryset to get the queryset before reducing it down to a single item. From my understanding of the get_queryset method, it seems to be a pre-process optimzation stage to reduce/specialize the query depending on whether it is a list or not.

  7. Can we change this to use integration-id ?

  8. reviewboard/webapi/resources/configured_integration.py (Diff revision 7)
     
     
     
     
     
     

    See previous comment about docstrings.

  9. You can use enabled as a key-word argument to the function to not have to look into kwargs.

  10. "Delete a configured integration."

  11. Can we just do the call to get_integration_manager inside the class's __init__ method?

    Are we ever expecting to instantiate this with a different integration manager?

  12. reviewboard/webapi/resources/integration.py (Diff revision 7)
     
     
     
     

    Can we pull the query set out into a variable?

  13. Same comment here re: integration managers as in the previous resource.

  14. 
      
Xuanyi Lin
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/root.py
        reviewboard/integrations/urls.py
        reviewboard/webapi/resources/__init__.py
        reviewboard/webapi/tests/test_configured_integration.py
        reviewboard/integrations/views.py
        reviewboard/webapi/resources/configured_integration.py
        reviewboard/webapi/resources/integration.py
        reviewboard/urls.py
        reviewboard/webapi/tests/test_integration.py
        reviewboard/integrations/admin.py
    
    
  2. 
      
David Trowbridge
  1. Some of the parts of this change look like they'd be more appropriate for /r/7106 (like the admin.py, urls.py, and views.py changes)

  2. No blank line here.

  3. All str in here should be six.text_type for future Python 3 compatibility. Here and below.

  4. I think I'd prefer if these conditionals were switched:

    if config and config.integration:
        return config.integration.name
    else:
        return None
    
  5. Same as above.

  6. reviewboard/webapi/resources/configured_integration.py (Diff revision 8)
     
     
     
     
     
     

    Same as above.

  7. Same as above.

  8. This should have a docstring.

  9. This should have a docstring.

  10. No blank line here.

  11. All str items in here should be six.text_type

  12. reviewboard/webapi/resources/integration.py (Diff revision 8)
     
     
     
     
     
     

    Please switch the conditional to test if integration: and return None in the else clause.

  13. This should have a docstring.

  14. 
      
Xuanyi Lin
Review request changed

Status: Discarded

Change Summary:

Closed in favor of a revised version of Integrations that landed for 3.0.

Loading...