feat: leveldb #44
feat: leveldb #44
Conversation
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
| throw new Error( | ||
| "Error: Instantiation failed: Use DBService.getInstance() instead of new." | ||
| ); | ||
| } |
There was a problem hiding this comment.
What about making it a singleton?
If the instance already exists just return it rather than throwing
| @@ -11,8 +10,18 @@ export interface RunOptions extends WatchtowerOptions { | |||
| rpc: string[]; | |||
| deploymentBlock: string[]; | |||
There was a problem hiding this comment.
why was the deploymentBlock an array?
is this how you plan to run it in multiple networks? If so, in principle, it feels a better idea to use separated instances
|
|
||
| export type DBLevel = Level<string, string>; | ||
|
|
||
| export default class DBService { |
There was a problem hiding this comment.
nit: This name is a bit generic (but it depeds on Level db).
It would be great if you can use the Repository or DAO pattern to abstract the persistance. This way, this implementation will use Level DB, but others could use anything else.
This could have a repository or DAO layer, and then you create a ContextRepository interface, with a ContextRepositoryLevel or ContextRepositoryImpl (since we don't have more)
| } | ||
|
|
||
| public getDB() { | ||
| return this.db; |
There was a problem hiding this comment.
If you use the repository pattern, you would not expose this, but would provide for the service layer the basic CRUD operations you need
| } | ||
|
|
||
| export type SingularRunOptions = Omit<RunOptions, "rpc" | "deploymentBlock"> & { |
There was a problem hiding this comment.
I don't think we should have "plural" TBH, this should be the default run
Description
Given that we are already using a NoSQL database and we need some storage, I dropped in
leveldbas the NoSQL solution. This allows us to eliminate all external dependencies, making it friendlier for spinning up in bandwidth constrained environments (ie. hackathons), and reduces complexity with deployment (no requirement for external SQL servers).Changes
levelpackages and `ts.dbsingleton for holding the database.commander.awaitthat were dependent onsecretlookups previously.How to test
No testing for this PR, besides
eslintandprettierpassing, excluding to-be-deprecated actions. This PR is mostly for "readability".Related Issues
Closes #40