Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix database move message #5506

Merged

Conversation

benjaoming
Copy link
Contributor

Summary

The current setup command wrongly claims that a database file exists and will be backed up on a clean release:

$ rm -rf ~/.kalite
$ kalite start --foreground
Running 'kalite start' in foreground...
Generating a new secret key file ~/.kalite/secretkey.txt...

Setting up KA Lite; this may take a few minutes; please wait!

                                     
   _   __  ___    _     _ _          
  | | / / / _ \  | |   (_) |         
  | |/ / / /_\ \ | |    _| |_ ___    
  |    \ |  _  | | |   | | __/ _ \   
  | |\  \| | | | | |___| | ||  __/   
  \_| \_/\_| |_/ \_____/_|\__\___|   
                                     
https://learningequality.org/ka-lite/
                                     
         version 0.17.2
                                     
(Re)moving database file to temp location, starting clean install. Recovery location: /tmp/tmpkPYEPM

@@ -398,7 +395,7 @@ def handle(self, *args, **options):
########################

# Move database file (if exists)
if install_clean and database_file and os.path.exists(database_file):
if install_clean and database_file and database_exists:

This comment was marked as spam.

@codecov
Copy link

codecov bot commented Aug 19, 2017

Codecov Report

Merging #5506 into 0.17.x will decrease coverage by 0.01%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           0.17.x    #5506      +/-   ##
==========================================
- Coverage   62.77%   62.76%   -0.02%     
==========================================
  Files         117      117              
  Lines        6555     6553       -2     
==========================================
- Hits         4115     4113       -2     
  Misses       2440     2440
Impacted Files Coverage Δ
kalite/distributed/management/commands/setup.py 43.16% <25%> (-0.49%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7868918...cc760cf. Read the comment docs.

Simplify some of the reasoning around whether a database exists at a clean install
@benjaoming benjaoming force-pushed the fix-database-move-message branch from 6eb3ddb to cc760cf Compare August 19, 2017 18:24
@benjaoming
Copy link
Contributor Author

@mrpau-eugene could you check if I got it right? It's a bit complicated, would be nice with another pair of eyes to verify :)

Copy link
Contributor

@mrpau-eugene mrpau-eugene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

if not options["interactive"] \
or raw_input_yn("Keep database file and upgrade to KA Lite version %s? " % VERSION) \
or not raw_input_yn("Remove database file '%s' now? " % database_file) \
or not raw_input_yn("WARNING: all data will be lost! Are you sure? "):

This comment was marked as spam.

Copy link
Contributor Author

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-reviewed again, still looks good

@benjaoming benjaoming added this to the 0.17.6 milestone Sep 7, 2019
@benjaoming benjaoming merged commit e269639 into learningequality:0.17.x Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants