fix(sparql-qlever): unify mountDir/cwd into dataDir and fix NativeTaskRunner bugs#239
Merged
fix(sparql-qlever): unify mountDir/cwd into dataDir and fix NativeTaskRunner bugs#239
Conversation
- Add RunOptions with optional cwd to TaskRunner.run() interface - NativeTaskRunner respects per-command cwd override - Importer passes dirname(file) so QLever finds downloaded files - Fixes 'No such file or directory' in native mode where the process CWD didn't match the download directory
ddeboer
force-pushed
the
fix/qlever-native-cwd
branch
from
855a20c to
83ede23
Compare
ddeboer
changed the title
…lation - ?? has lower precedence than +, so only the first chunk of stdout/stderr was ever stored; subsequent chunks were silently discarded. Add parentheses to accumulate correctly. - Fix taskOutput() appending literal 'undefined' when stderr Map has no entry. - Rename RunOptions to TaskRunOptions to avoid collision with @lde/pipeline's RunOptions.
ddeboer
changed the title
…ions - Replace mode-specific mountDir (docker) and cwd (native) with shared dataDir property - createQlever passes dataDir as mountDir to DockerTaskRunner, as cwd to NativeTaskRunner - Accept optional downloader and pass it through to Importer - Revert TaskRunner interface: remove TaskRunOptions, revert run() to run(command: string) - Keep pre-existing bug fixes in NativeTaskRunner (operator precedence, taskOutput)
ddeboer
changed the title
…k runner cwd match
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mountDir/cwdintodataDironQleverOptions: the old discriminated union splitmountDir(Docker-only) andcwd(native-only) into separate branches, so callers passingmountDirhad it silently ignored in native mode. The new shareddataDirproperty is passed asmountDirtoDockerTaskRunnerand ascwdtoNativeTaskRunner.downloaderoncreateQleverand pass it through toImporter, instead of always using the default.NativeTaskRunneroutput accumulation:?? '' + dataonly concatenated on the first chunk due to+binding tighter than??.taskOutput()appending literal"undefined"when stderr was empty.Breaking change:
QleverOptions.mountDirandQleverOptions.cwdare replaced byQleverOptions.dataDir.