Conversation
| @sequencerModule() | ||
| @closeable() | ||
| @dependencyFactory() | ||
| export class InMemorySequencerCoreModule extends SequencerModule<InMemorySequencerCoreConfig> { |
There was a problem hiding this comment.
We should change this naming, bcs this has nothing to do with InMemory itself.
| /** | ||
| * Returns an object containing the keys listed in ownConfigKeys. | ||
| */ | ||
| public get ownConfig(): OwnConfig { |
There was a problem hiding this comment.
In general, I think this is good, a few notes:
Let's replace the ownConfigKeys mechanism by maing the OwnConfig type transform to { ownConfig: OwnConfig } when passing it to ConfigurableModule (in the intersection type CombinedModuleContainerConfig) and then just accessing this.config.ownConfig
There was a problem hiding this comment.
Also let's rename ownConfig to containerConfig maybe? Or is that too misleading?
| await this.startServer(); | ||
| } | ||
|
|
||
| // Server logic |
There was a problem hiding this comment.
Just a thought - instead of flattening the Graphql implementation into here. Could we not just make this class a dependencyfactory creating the GraphqlServer? This way users could override it by passing a GraphqlServer module if they don't want to use Koa for some reason
|
|
||
| public async start(): Promise<void> { | ||
| this.sequencerStartupModule.config = this.config.SequencerStartupModule; | ||
| this.blockProducerModule.config = this.config.BlockProducerModule; |
There was a problem hiding this comment.
This works? I would've assumed that we should do this in create()...
|
|
||
| public static dependencies(): InMemorySequencerCoreDependencies { | ||
| return { | ||
| sequencerStartupModule: { |
There was a problem hiding this comment.
What's up with the lowercase here? We normally do Uppercase for these in the codebase
| } | ||
|
|
||
| public async close(): Promise<void> { | ||
| await this.sequencerStartupModule.close(); |
There was a problem hiding this comment.
Doubt that you need to do that - modulecontainer should pick up that sequencerStartupModule is Closeable and automatically call close() on it
| @@ -14,7 +13,7 @@ export type DatabasePruneConfig = { | |||
|
|
|||
| @sequencerModule() | |||
There was a problem hiding this comment.
Remove this annotation + Sequencermodule Superclass
2 participants
closes #458