-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
Allows LDWorkbench to run multiple parallel generators for one iterator #4 #25
Conversation
Adds parallel approach to LDWorkbench with base of feature/3/mightymax branch
09edbf7
to
8f9a0d2
Compare
const valuePatterns: ValuePatternRow[] = [] | ||
for (const $this of this.$thisList) { | ||
this.iterationsProcessed++ | ||
const group: GroupPattern = { type: 'group', patterns: [...unionQuery.where ?? []] } | ||
group.patterns.unshift(this.$thisToBind($this)) | ||
union.patterns.push(group) | ||
valuePatterns.push({'?this': $this}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this more readable / concise with a map
? If you think writing out the for-loop is more readable, you can resolve this comment without making any changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I only now read your slack message. You can also count inside of the map, but just do whatever you think is most understandable.
{ | ||
/** | ||
* Path (prefixed with "file://") or SPARQL Query | ||
* that makes the generator using SPARQL construct. | ||
*/ | ||
query: string; | ||
/** | ||
* The SPARQL endpoint for the generator. | ||
* If it starts with "file://", a local RDF file is queried. | ||
* If ommmitted the endpoint of the Iterator is used. | ||
*/ | ||
endpoint?: string; | ||
/** | ||
* Overrule the generator's behaviour of fetching results for 10 bindings of $this per request. | ||
*/ | ||
batchSize?: number; | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would name this type s.t. you don't have to repeat it. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah this is automatically generated from the JSON schema here: https://github.com/netwerk-digitaal-erfgoed/ld-workbench/blob/main/static/ld-workbench.schema.json
|
||
generator.on('end', (iterationsIncoming, statements, processed) => { | ||
this.totalProcessed += processed | ||
if (this.totalProcessed >= (iterationsIncoming * this.configuration.generator.length)){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you reference this.configuration.generator
, while this is part of a forEach
loop over this.generators
. Is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, since I want to know the length (the number of generators) for this stage, since another stage might have a different number of generators, hence I look for the stages configuration to see how many generators are present.
] | ||
} | ||
const pipeline = new Pipeline(configuration, {silent: true}) | ||
const stageConfig = configuration.stages[0] | ||
const stage = new Stage(pipeline, stageConfig) | ||
const generator = new Generator(stage) | ||
const generator = new Generator(stage, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the 0
mean?
Or more general: if I can't easily determine what the 0
means here, the arguments of the constructor of Generator
should maybe be changed to an options object, s.t. the caller names the arguments (and the reader of the calling code can then read the names).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0
in this case is the index of the generator, given the stage. A stage can for example have 2 generators, this index specifies which one of the generators it is, and which configuration from this stage the Generator class should use.
In the config, the
generator
key should now be specified with an array of different generator configurations, which will result in running these generators in parallel for that iterator.