Djblets.extensions.TemplateHook applies_to() sandboxing

Review Request #6545 — Created Nov. 1, 2014 and submitted

justy777
Djblets
master
f27a497...
djblets, students

Extensions that create a TemplateHook subclass with a new applies_to function can throw an exception inside Djblets. The part of Djblets that calls that method has been sandboxed.

Now when a TemplateHook subclass's applies_to() function from an extension throws an exception; Djblets logs the error.

Unit test has been written to make sure that applies_to() from a TemplateHook subclass has been called, and when an exception is thrown it gets logged.

The test fails without the sanboxing, and succeeds with it.

Description From Last Updated

Space should go on the first line instead of the second.

chipx86chipx86

Can we also check the resulting string? That'll help make sure we're not unintentionally showing some big Django traceback.

chipx86chipx86

The indentation here broke the logic. It used to be that the "try: yield" was inside the hook.applies_to conditional. Now ...

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/extensions/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/extensions/tests.py
    
    
  2. 
      
justy777
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/extensions/templatetags/djblets_extensions.py
        djblets/extensions/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/extensions/templatetags/djblets_extensions.py
        djblets/extensions/tests.py
    
    
  2. 
      
chipx86
  1. Looks good! Couple small things.

  2. Space should go on the first line instead of the second.

  3. djblets/extensions/tests.py (Diff revision 2)
     
     

    Can we also check the resulting string? That'll help make sure we're not unintentionally showing some big Django traceback.

    1. What should I check it for? or compare it to?

  4. 
      
justy777
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/extensions/templatetags/djblets_extensions.py
        djblets/extensions/tests.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/extensions/templatetags/djblets_extensions.py
        djblets/extensions/tests.py
    
    
  2. 
      
brennie
  1. Ship It!

  2. 
      
david
  1. 
      
  2. djblets/extensions/templatetags/djblets_extensions.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    The indentation here broke the logic. It used to be that the "try: yield" was inside the hook.applies_to conditional. Now that will only get called in the case that applies_to raises an exception.

  3. 
      
justy777
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/extensions/templatetags/djblets_extensions.py
        djblets/extensions/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/extensions/templatetags/djblets_extensions.py
        djblets/extensions/tests.py
    
    
  2. 
      
david
  1. Ship It!

  2. 
      
justy777
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (5e31f68)
Loading...