Fix issues upgrading SCMTool IDs on non-pristine setups.

 






 

 


  Review Request #12380
  —
  Created June 17, 2022 and submitted 

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-5.0.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
While trying to upgrade my system to the new registry-backed SCMTool

support, a number of issues with the new upgrade process for SCMTools

0were discovered, in part due to the complexity and history of my local

database.


These issues include:


    

  1. 

    Pre-upgrade only recorded state to upgrade if the
    
repository_scmtool_id was not yet in the database, preventing a
    
   re-attempt at upgrading from working on failure (and outright
    
   skipping the pre-upgrade work if running evolve --execute before
    
   running a managed upgrade).
    

    2. 

  2. 

    SCMTools that could not be loaded (missing package) would cause the
    
   pre-upgrade steps to crash and no IDs to be migrated.
    

    3. 

  3. 

    Integration configurations that lacked a saved conditions field
    
   would cause a lookup on conditions to try to load the integration
    
   class in order to retrieve the default, and as these are provided by
    
   unloaded extensions, this would cause the pre-upgrade steps to
    
   crash.
    

    4. 



This change fixes all three scenarios.


Pre-upgrade steps for SCMTools are now run if either the evolution is

missing from the database or if we can find any repositories lacking

an ID. This enables re-attempts in both the premature evolve --execute

case and in the upgrade re-attempt case.


We now check for exceptions on getting the SCMTool IDs. If we fail to

load any, we'll list them and follow up with some helpful instructions.

These repositories will remain in their pre-upgrade state until resolved

(an upcoming change will improve upon this bad state).


For updates to integration configuration, the code is now careful to

work directly with the settings dictionary rather than the wrapper

methods, to avoid an attempted load of the integration. As an

optimization, the queryset also avoids loading any more state than

needed for the upgrade.

 








  


 
 

  
Tested an upgrade on a clean database.


Tested an upgrade on my messy database, with missing SCMTools and

integration configurations both without explicit conditions saved

and without a ConditionsField.


Verified that the IDs got migrated over even when errors were

encountered.


After resolving the errors, I verified that the upgrade could be

re-attempted and the remaining SCMTool IDs were looked up and

migrated over.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Fix issues upgrading SCMTool IDs on non-pristine setups.
      
While trying to upgrade my system to the new registry-backed SCMTool
support, a number of issues with the new upgrade process for SCMTools
0were discovered, in part due to the complexity and history of my local
database.

These issues include:

1. Pre-upgrade only recorded state to upgrade if the
   `repository_scmtool_id` was not yet in the database, preventing a
   re-attempt at upgrading from working on failure (and outright
   skipping the pre-upgrade work if running `evolve --execute` before
   running a managed upgrade).

2. SCMTools that could not be loaded (missing package) would cause the
   pre-upgrade steps to crash and no IDs to be migrated.

3. Integration configurations that lacked a saved `conditions` field
   would cause a lookup on `conditions` to try to load the integration
   class in order to retrieve the default, and as these are provided by
   unloaded extensions, this would cause the pre-upgrade steps to
   crash.

This change fixes all three scenarios.

Pre-upgrade steps for SCMTools are now run if either the evolution is
missing from the database *or* if we can find any repositories lacking
an ID. This enables re-attempts in both the premature `evolve --execute`
case and in the upgrade re-attempt case.

We now check for exceptions on getting the SCMTool IDs. If we fail to
load any, we'll list them and follow up with some helpful instructions.
These repositories will remain in their pre-upgrade state until resolved
(an upcoming change will improve upon this bad state).

For updates to integration configuration, the code is now careful to
work directly with the `settings` dictionary rather than the wrapper
methods, to avoid an attempted load of the integration. As an
optimization, the queryset also avoids loading any more state than
needed for the upgrade.

     


    		
    1c87f7d931eebc34345eb9f3eefc5095ea053008

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
This blank line isn't necessary.

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
I feel like we should print what tools are missing.

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
undefined name 'e' Column: 28 Error code: F821

      		
      
       reviewbotreviewbot
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/upgrade.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    This blank line isn't necessary.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/upgrade.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    Does this actually work when the scmtool_id field doesn't yet exist?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      No, but if has_evolution is False, we won't hit that code.
      

      I could wrap the whole thing in an exception as an extra measure of protection.
      
 
      

      2. 


 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/upgrade.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    I feel like we should print what tools are missing.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  5. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  








 
Change Summary:

 
    

  • Removed a blank line.
    • 

  • Added extra protection in case a database query fails when determining whether to upgrade tools.
    • 

  • Changed the error output for tools that fail to load. This is built to be expanded in an upcoming change.
    • 






 



  

   
Commits:

   
    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Fix issues upgrading SCMTool IDs on non-pristine setups.
      
While trying to upgrade my system to the new registry-backed SCMTool
support, a number of issues with the new upgrade process for SCMTools
0were discovered, in part due to the complexity and history of my local
database.

These issues include:

1. Pre-upgrade only recorded state to upgrade if the
   `repository_scmtool_id` was not yet in the database, preventing a
   re-attempt at upgrading from working on failure (and outright
   skipping the pre-upgrade work if running `evolve --execute` before
   running a managed upgrade).

2. SCMTools that could not be loaded (missing package) would cause the
   pre-upgrade steps to crash and no IDs to be migrated.

3. Integration configurations that lacked a saved `conditions` field
   would cause a lookup on `conditions` to try to load the integration
   class in order to retrieve the default, and as these are provided by
   unloaded extensions, this would cause the pre-upgrade steps to
   crash.

This change fixes all three scenarios.

Pre-upgrade steps for SCMTools are now run if either the evolution is
missing from the database *or* if we can find any repositories lacking
an ID. This enables re-attempts in both the premature `evolve --execute`
case and in the upgrade re-attempt case.

We now check for exceptions on getting the SCMTool IDs. If we fail to
load any, we'll list them and follow up with some helpful instructions.
These repositories will remain in their pre-upgrade state until resolved
(an upcoming change will improve upon this bad state).

For updates to integration configuration, the code is now careful to
work directly with the `settings` dictionary rather than the wrapper
methods, to avoid an attempted load of the integration. As an
optimization, the queryset also avoids loading any more state than
needed for the upgrade.

     


    		
    b546e75de0b11264a5ca4fec539097d255aa89b7

   




   


    

    

     

      Fix issues upgrading SCMTool IDs on non-pristine setups.
      
While trying to upgrade my system to the new registry-backed SCMTool
support, a number of issues with the new upgrade process for SCMTools
0were discovered, in part due to the complexity and history of my local
database.

These issues include:

1. Pre-upgrade only recorded state to upgrade if the
   `repository_scmtool_id` was not yet in the database, preventing a
   re-attempt at upgrading from working on failure (and outright
   skipping the pre-upgrade work if running `evolve --execute` before
   running a managed upgrade).

2. SCMTools that could not be loaded (missing package) would cause the
   pre-upgrade steps to crash and no IDs to be migrated.

3. Integration configurations that lacked a saved `conditions` field
   would cause a lookup on `conditions` to try to load the integration
   class in order to retrieve the default, and as these are provided by
   unloaded extensions, this would cause the pre-upgrade steps to
   crash.

This change fixes all three scenarios.

Pre-upgrade steps for SCMTools are now run if either the evolution is
missing from the database *or* if we can find any repositories lacking
an ID. This enables re-attempts in both the premature `evolve --execute`
case and in the upgrade re-attempt case.

We now check for exceptions on getting the SCMTool IDs. If we fail to
load any, we'll list them and follow up with some helpful instructions.
These repositories will remain in their pre-upgrade state until resolved
(an upcoming change will improve upon this bad state).

For updates to integration configuration, the code is now careful to
work directly with the `settings` dictionary rather than the wrapper
methods, to avoid an attempted load of the integration. As an
optimization, the queryset also avoids loading any more state than
needed for the upgrade.

     


    		
    2a7290c6b4553c6adfbc38a31a94c95717cb91a8

   



  
 







   		
  




  

   
Diff:

   
    
 Revision 2 (+250 -52)
Show changes
 
    reviewboard/upgrade.py 
 

   		
  



 









 

  
Checks run (1 failed, 1 succeeded)



  

 flake8
 failed.
 





  

 JSHint
 passed.
 





 







 
flake8

 


 















  

 
















 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to release-5.0.x (b5337d4)





 


 










  

 









     

    

   

  

  

   
   Loading...