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

Remove Memory database #607

Closed
thesimplekid opened this issue Feb 18, 2025 · 2 comments · Fixed by #613
Closed

Remove Memory database #607

thesimplekid opened this issue Feb 18, 2025 · 2 comments · Fixed by #613
Assignees
@crodas
Labels
mint wallet

Comments

@thesimplekid
Copy link
Collaborator

We've discussed removing the memory database from the mint database. In cases where an in memory database is desired an in memory sql db could be used and it removed the need for us to maintain another db within cdk. I think this is fairly obvious for the mint should be also do it for the wallet?

cc @crodas
@crodas
Copy link
Contributor

crodas commented Feb 18, 2025

I have started working on it and will submit a PR shortly. In my PR, SQLite with :memory: is being used, with migration every time the Memory Database is being created.

The main benefit would be fewer things to maintain, specially when transactions changes are added to the database, it will be implemented already by the SQLite driver.

Does this sound like a plan?

@thesimplekid
Copy link
Collaborator Author

Does this sound like a plan?

Yes I'm happy we do this for both mint and wallet unless, anyone thinks we should keep it?

@thesimplekid thesimplekid mentioned this issue Feb 18, 2025
Draft
2 tasks
crodas added a commit to crodas/cdk that referenced this issue Feb 23, 2025
@crodas
Drop the in-memory database
9f714c6 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
@crodas crodas mentioned this issue Feb 23, 2025
Merged
2 tasks
crodas added a commit to crodas/cdk that referenced this issue Feb 23, 2025
@crodas
Drop the in-memory database
e7c3161 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
crodas added a commit to crodas/cdk that referenced this issue Feb 23, 2025
@crodas
Drop the in-memory database
1930988 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
crodas added a commit to crodas/cdk that referenced this issue Feb 24, 2025
@crodas
Drop the in-memory database
1ff4f7b 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
crodas added a commit to crodas/cdk that referenced this issue Feb 24, 2025
@crodas
Drop the in-memory database
aeae058 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
crodas added a commit to crodas/cdk that referenced this issue Feb 27, 2025
@crodas
Drop the in-memory database
29e1e97 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
crodas added a commit to crodas/cdk that referenced this issue Feb 28, 2025
@crodas
Drop the in-memory database
f03ef4d 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
crodas added a commit to crodas/cdk that referenced this issue Feb 28, 2025
@crodas
Drop the in-memory database
aa2fb12 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
crodas added a commit to crodas/cdk that referenced this issue Mar 4, 2025
@crodas
Drop the in-memory database
c6118b1 
Fixes cashubtc#607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance
thesimplekid pushed a commit that referenced this issue Mar 4, 2025
@crodas
Drop the in-memory database (#613)
f7d9a1b 
* Drop the in-memory database

Fixes #607

This PR drops the implementation of in-memory database traits.

They are useful for testing purposes since the tests should test our codebase
and assume the database works as expected (although a follow-up PR should write
a sanity test suite for all database trait implementors).

As complexity is worth with database requirements to simplify complexity and
add more robustness, for instance, with the following plans to add support for
transactions or buffered writes, it would become more complex and
time-consuming to support a correct database trait. This PR drops the
implementation and replaces it with a SQLite memory instance

* Remove OnceCell<Mint>

Without this change, a single Mint is shared for all tests, and the first tests
to run and shutdown makes the other databases (not-reachable, as dropping the
tokio engine would also drop the database instance).

There is no real reason, other than perhaps performance. The mint should
perhaps run in their own tokio engine and share channels as API interfaces, or
a new instance should be created in each tests

* Fixed bug with foreign keys

[1] https://gist.github.com/crodas/bad00997c63bd5ac58db3c5bd90747ed

* Show more debug on failure

* Remove old code

* Remove old references to WalletMemoryDatabase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@crodas crodas

Labels
mint wallet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Drop the in-memory database crodas/cdk
2 participants
@crodas @thesimplekid