potiuk commented on issue #4488: [AIRFLOW-3678] Fixes 'airflow resetdb' for 
initialized Postgres DB
URL: https://github.com/apache/airflow/pull/4488#issuecomment-453743161
 
 
   OK. I know the exact scenario it happened and I know why you have not seen 
it, and I think my fix is the *correct* workaround in fact.
   
   The scenario:
   - We run our tests in our own Docker environment which has been prepared 
before dag_stat table was removed. This means that the table is there, created 
in the DB. 
   - We update to latest version of the code and rather than upgradedb we run 
resetdb - just to make sure that the database is "fresh"
   - If we would run upgradedb before the dag_stat table would have been 
deleted by migration script. The problem is that resetdb drops only the 
"current" tables - not those that we have in the database but those which are 
in the latest version of the code. That's why dag_stat table was not deleted on 
reset and subsequently failed when it was created with old migration script.
   
   This is pretty much heavy corner case that depends on many things to happen 
(old version of db, new version of code and running resetdb without earlier 
upgrade). However I think it is somehow valid. I think you should expect that 
resetdb will actually recreate the database no matter what. Running upgradedb 
first should not be a prerequisite.
   
   I don't think we should remove old migrations, it's quite disruptive and it 
can cause some side effects - for example the dag_stat table might remain in 
someone's database forever in the case described above. 
   
   The conditional creation of the table which I implemented actually solves 
the problem nicely. Resetdb will eventually drop the table when the migration 
is complete and desired state of the DB is as expected. Otherwise it is 
harmless. Even if you decide to recreate the table later on with the same name 
- everything will be fine - the table will dropped on migration and recreated.
   
   I think it might simply be a good practice to do conditional creation of 
tables (I saw several other tables are created in current_schema for one) and 
for sure when we decide to drop a table we should add this conditional creation 
in the migration where the table is created and it will solve the problem.
   
   I have amended my proposal and I added such conditional creation for all 
tables.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to