bulkhead #7
bulkhead #7
Conversation
| private readonly limit: number = Infinity, | ||
| ) { } | ||
|
|
||
| public store(data: ExecutionMetaData<T>): this { |
There was a problem hiding this comment.
Please use standard queue method names, like push and pop instead of store and next
| @@ -0,0 +1,14 @@ | |||
| import { Factory } from '../../interfaces/factory'; | |||
There was a problem hiding this comment.
Chances are low we will ever have another execution queue implementation than the current one. Therefore the factory here seems like an added complexity. I'd suggest rather to delete this file, and implement a static method static create<T>(limit: number): ExecutionQueue<T> directly in the ExecutionQueue class.
| @@ -0,0 +1,57 @@ | |||
| import { ExecutionQueue } from '../ExecutionQueue/ExecutionQueue'; | |||
There was a problem hiding this comment.
Bulkhead/Bulkhead.ts doesn't sound nice. Maybe just bulkhead/index.ts ?
| @@ -0,0 +1,18 @@ | |||
| import { Factory } from '../../interfaces/factory'; | |||
There was a problem hiding this comment.
Same as for queue implementation, this factory is not of much use taking into account there won't be another bulkhead implementations. Now it looks like an added complexity. I'd suggest that we implement a static create method directly in the bulkhead class.
|
|
||
| const result = method(...args); | ||
|
|
||
| const afterExecution = async () => { |
There was a problem hiding this comment.
Not sure if async is needed here.
| private async execute(method: (...args: any[]) => Promise<T>, args: any[]) { | ||
| this.inExecution += 1; | ||
|
|
||
| const result = method(...args); |
There was a problem hiding this comment.
The provided method might throw an error, thus breaking our queue execution.
| this.callNext(); | ||
| }; | ||
|
|
||
| result.then(afterExecution, afterExecution); |
There was a problem hiding this comment.
What if the method never resolves? It will stay forever in the queue and potentially might block the queue. I think we might need an additional option to setup a timeout for the execution. Could we use the @timeout from current library? Either as a decorator, or directly as a method.
No description provided.