Fix some remaining issues with WebHook key types in payloads.

 






 

 


  Review Request #10348
  —
  Created Dec. 5, 2018 and submitted 

 


















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-3.0.x
 









 
 

  
 









 
 

  
 









 
 

  b53c2c3...
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
A recent change fixed several type-related issues in WebHook payloads,

but str types for keys were still problematic. This change addresses

this, and at the same time prevents future issues with subclasses of

primitive types, OrderedDicts, and byte strings containing Unicode

content. It also keeps order preserved for dictionaries, which helps

with consistent payloads (useful for testing).

 








  


 
 

  
Unit tests pass.

 







 









 





  








 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
E128 continuation line under-indented for visual indent

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
I feel this warrants a comment because I don't know the interaction here between calling `six.text_type` on the result of …

      		
      
       brenniebrennie
      
      
       
      
     




     

      
       
       
Here too

      		
      
       brenniebrennie
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




brennie




   

   









  

 



 

  


   

   

    
 
brennie


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/notifications/webhooks.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    I feel this warrants a comment because I don't know the interaction here between calling `six.text_type` on the result of `force_text`.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Good point. So the reason for this is that force_text() on a SafeText gives a SafeText, so we need to cast it back to a standard string. I'll make the appropriate comment.
      
 
      

      2. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/notifications/webhooks.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Here too
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to release-3.0.x (ad2d858)