Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

[WIP] Use one database for all wallets #1581

Closed
wants to merge 1 commit into from
Closed

[WIP] Use one database for all wallets #1581

wants to merge 1 commit into from

Conversation

jsaur
Copy link
Contributor

@jsaur jsaur commented Apr 8, 2019

Signed-off-by: Jacob Saur jsaur@kiva.org

This restructures the postgres storage plugin to use one database for all wallets, rather than a separate database for each wallet.
For your review @ianco

Signed-off-by: Jacob Saur <jsaur@kiva.org>
@ianco
Copy link
Contributor

ianco commented Apr 8, 2019

Thanks @jsaur this code is very good!

Regarding library initialization I see 3 options:

  1. Have the library loader call the init function to create the database
  2. Check if the database exists on wallet create, and create it if necessary (requires the create wallet function to have permissions to create database)
  3. Have the postgres admin create the database in advance

Number 2 is sort of how it works now (database per wallet)

I also suggest keeping the existing scheme (one database per wallet) and make a configuration parameter so the postgres storage can be initialized to use either method (database per wallet or one database containing all wallets).

@jovfer jovfer changed the title Use one database for all wallets [WIP] Use one database for all wallets Apr 24, 2019
@jsaur
Copy link
Contributor Author

jsaur commented Apr 24, 2019

Talking it over with @ianco we want to allow both one wallet per database and all wallets in one database, which will probably involve a structure with one parent class and two children. Going to resume this again in late May.

@jovfer jovfer added the paused label May 10, 2019
@jovfer
Copy link
Contributor

jovfer commented May 10, 2019

@jsaur please remove "paused" label as soon as you resume work on this PR

@cam-kiva
Copy link

@jovfer This is Cam (I'll be back on my normal github account when I am back in salt lake this should be temporary ). But we can remove the paused tag now... I will be working on this

@jovfer jovfer removed the paused label May 24, 2019
@jsaur
Copy link
Contributor Author

jsaur commented Jul 9, 2019

@ianco has a better PR out there so closing this in favor of his

@jsaur jsaur closed this Jul 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants