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.

Sign up for GitHub

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

Jump to bottom

make sure to close and destroy the database before removing files from disk #1192

Merged
merged 2 commits into from
Dec 16, 2019
Merged

make sure to close and destroy the database before removing files from disk #1192

merged 2 commits into from
Dec 16, 2019

Conversation

KtorZ
Copy link
Member

@KtorZ KtorZ commented Dec 16, 2019

Issue Number

#1071

Overview

  • I have made sure to close and destroy the database before removing files from disk

Comments

@KtorZ KtorZ self-assigned this Dec 16, 2019
KtorZ
KtorZ commented Dec 16, 2019
View reviewed changes
lib/core/src/Cardano/Wallet/DB/Sqlite.hs
, databaseFile wid <> "-wal"
, databaseFile wid <> "-shm"
]
readMVar mvar >>= destroyDBLayer
Copy link
Member Author

Choose a reason for hiding this comment

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

That's the main change here. Make sure the db is closed before we attempt to remove the files from the disk ^.^ ...

@piotr-iohk piotr-iohk mentioned this pull request Dec 16, 2019
Closed
1 task
paweljakubas
paweljakubas approved these changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

LGTM!. It would be good to have ultimate proof here, ie., have a test that really checks that, after clearing a database, the files are gone

@KtorZ
Copy link
Member Author

KtorZ commented Dec 16, 2019

bors r+

iohk-bors bot added a commit that referenced this pull request Dec 16, 2019
@iohk-bors @KtorZ
Merge #1192
09ff1dc 
1192: make sure to close and destroy the database before removing files from disk r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#1071 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have made sure to close and destroy the database before removing files from disk

# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Dec 16, 2019

Build succeeded

@iohk-bors iohk-bors bot merged commit 4179807 into master Dec 16, 2019
@KtorZ KtorZ deleted the KtorZ/1071/db-file-cleanup branch December 30, 2019 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paweljakubas paweljakubas paweljakubas approved these changes

Assignees

@KtorZ KtorZ

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KtorZ @paweljakubas