fix 'NoneType' object has no attribute 'scan_for_server' in rbt login/logout

 






 

 


  Review Request #7794
  —
  Created Dec. 3, 2015 and discarded 

 


















 

  

   Information

  





 
 

  tterrace
 









 
 

  RBTools
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  rbtools
 









 
 

  
 






 








 





  


 
 

  
rbt login/logout commands crash if the server url is set in the SCM config


The rbt login and logout commands do not initialize the scm tool like other

commands do. This means that if the server url is set in the scm (through 

.gitconfig for example), rather than the command line then login and logout

will crash with an AttributeError.

 








  


 
 

  
Tests pass except for the MercurialClient (+svn) tests, where I'm seeing this issue with hgsubversion: https://bitbucket.org/durin42/hgsubversion/issues/449/extension-breaks-mercurial-36-rc2


Additionally, it seems the MercurialClient (+svn) test failure will leave the suite in a bad state because all subsequent tests will fail with an OSError, but when run in isolation they pass just fine. The travis-ci build hit the same issue: https://travis-ci.org/reviewboard/rbtools/jobs/94449505

 







 









 





  








 









 






  

 








 














 





 

  

   




reviewbot




   

   





  

 



 

  


   

   

    
 
reviewbot


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Tool: Pyflakes
Processed Files:
    rbtools/commands/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/__init__.py


    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   





 TT





   

   

  

 



 















 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    The fix seems like it'd work, but it feels like a bandaid for the real issue, which is that this is being called at all with a None value for the tool. That shouldn't be valid at all. Can you look at how we're getting to that state?
    

    Also, I'd like more info in the description (which we use as the commit message) as to the original problem being fixed and how it was caused.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Looks like the original cause was my fault. I think given the use case here, your fix is fine, but some documentation and function argument changes would do a lot to make this clearer down the road.
      

      Can you make the repository_url and tool arguments optional, and document in the docstring that, if not provided, this will only take into consideration the server URL provided on the comand line?
      

      RBTools docstrings are kind of a mess, but we're transitionining to the following format:
      

      

      """One-line summary.

Multi-line description.

Args:
    argname1 (argtype):
        Description

    argname2 (argtype, optional):
        Description

    ...

Returns:
    returntype:
    Description.
"""

      

      

      (In this case, both would have the ', optional' bit.)
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      (was writing this as you responded, take a look at the last part and let me know if you'd still like to keep login/logout from trying to initialize the scm)
      

      Okay I've updated the description with the root cause, hopefully it's clearer now.  You can reproduce the error by setting your server url in .gitconfig (or your scm of choice) and calling rbt login or logout. According to login.py the intention is to allow the user to login even if they're outside of the scm directory:
      

      ...allowing the user to log in and save a session cookie 
without needing to be in a repository or posting to
the server.

      

      One option is to do something like close.py does: attempt to initialize the scm only if the server url hasn't been set via .reviewboardrc or --server.
      
 
      

      3. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   





 TT





   

   

  

 



 















 



 

  

   





 TT





   

   

  

 



 















 





 

  

   




reviewbot




   

   





  

 



 

  


   

   

    
 
reviewbot


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/__init__.py



Tool: Pyflakes
Processed Files:
    rbtools/commands/__init__.py


    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Discarded

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...