feat: Generate insert and select methods for context object under db #177
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
| try { | ||
| const result = tables.reduce((prev, tableName) => ({ | ||
| ...prev, | ||
| [`insert_${tableName}`]: async (objects) => await this.insert(blockHeight, schemaName, tableName, objects), |
There was a problem hiding this comment.
naming: do we want camelCase for everything or it's fine to have underscores here?
There was a problem hiding this comment.
My idea was that since it's feasible for a developer to name their database something like test_9_table_4 or post_likes, I'd prefer to keep it as underscored simply to visually distinguish the modifier (insert/delete/select) while keeping the table names themselves consistent with how they're defined. It's not clear how we'd want to camelCase it otherwise. Would we want to remove underscores, uppercase the next letter after each one, and combine them (e.g. post_likes_and_comments -> PostLikesAndComments? Retain the underscores (e.g. insert_Post_likes_and_comments)? This is really more of a UX choice, but I'd say underscore and just the table name as is, is my personal vote on that.
runner/src/pg-client/pg-client.ts
Outdated
|
|
||
| constructor ( | ||
| connectionParams: ConnectionParams, | ||
| hasuraClient: HasuraClient = new HasuraClient(), |
There was a problem hiding this comment.
I think the correct abstraction here is to have DmlHandler take HasuraClient. HasuraClient also depends on PgClient, so now we actually have a circular dependency between the two.
Rather than having PgClient default to admin and then change as needed, we would instantiate new pg clients for each user. I think the same would be said for DmlHandler, we'd keep the API simple, and allow ourselves to configure it as needed. Perhaps we should structure it so:
DmlHandlertakes anaccount_idas input,DmlHandlerusesHasuraClientto fetch the connection infoDmlHandlerinstantiates a newPgClientwith those connection params
It's a good idea to think about where code makes the most sense, and what level of abstraction is best. PgClient is quite low level, so would make more sense to have the configuration a level above.
There was a problem hiding this comment.
Implemented! I've reduced pgclient back to its original form and moved it back out to the src folder. I updated hasura client to take the new function which returns connection parameters. I updated DmlHandler to take a dependency on hasura client and pgclient and refactored it to allow for unit testing. I implemented your suggestion of using class instances and using them to later initialize the class itself to allow for default initialization and mocking.
runner/src/indexer/indexer.ts
Outdated
| @@ -41,6 +44,7 @@ export default class Indexer { | |||
| fetch, | |||
| s3: new AWS.S3(), | |||
| provisioner: new Provisioner(), | |||
| dmlHandler: new DmlHandler(), | |||
There was a problem hiding this comment.
Following on about my comment around the right level of abstraction:
Because DmlHandler is ultimately user specific, it would make more sense to configure this at runtime. It should still be injected, so that we can mock it during testing, but we should create/configure it differently per indexer prior to executing it, i.e. in buildContext
There was a problem hiding this comment.
Took your suggestions to have a class instance as a dependency and lazy loading it when the insert/select statements are ran for the first time. The class itself is re-initialized each time the context object is rebuilt for each indexer function.
Generates table names from schema. Using the list of table names, it generates an insert and select method for each one. Methods are exposed through 'context.db'. Verified working through local testing against databases in docker image as well as frontend tested using debug mode.