Skip to content

Implement Sqlite.backup - #60

Open
joeybright wants to merge 2 commits into
gren-lang:mainfrom
joeybright:implement-sqlite-backup
Open

Implement Sqlite.backup#60
joeybright wants to merge 2 commits into
gren-lang:mainfrom
joeybright:implement-sqlite-backup

Conversation

@joeybright

Copy link
Copy Markdown
Contributor
  • Simple implementation and integration tests for the backup SQLite function
  • Using a builder pattern to allow specifying the rate of backup, allowing backup config to live in a variable separate from actually running it, and leave open any other future configuration options that we might want to add.
  • What isn't here: I tried to hook into the progress functionality provided by the SQLite node module, but ran into a lot of issues trying to get it to work with Gren streams. It's purely additive, though, and doesn't block the essential functionality that is working in this PR. We can add it in a later release, I think.

Implementation and integration tests for the `backup` SQLite function

@blaix blaix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, pending one test fixup. Will leave it to Robin for final approval.

Comment thread integration-tests/sqlite/src/Main.gren Outdated

@robinheghan robinheghan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Small nitpick, but otherwise it looks good!

I agree that we can figure out progress updates at a later stage.

Comment thread src/Sqlite.gren

runBackup : Backup -> Task Error {}
runBackup (Backup { destination, pages, db }) =
Gren.Kernel.Sqlite.backup (FileSystem.Path.toPosixString destination) pages db

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should move the toString call to kernel code, and use _FilePath_toString, which takes the current platform into account (generates win-paths on win-systems).

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.

3 participants