Develop #769
Merged
Develop #769
Conversation
This file contains 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
* Basic implementation of splitting platforms into executors and containers For now: - all docker platforms are containers - all native and nextflow platforms are executors the debug platform is just a platform should we add a docker executor? * define Executor and Container as a standalone trait * look for containerDirective in NextflowPlatform using getContainers * add id field to executor * add executorresources * add placeholder objects * Provide temporary wrapper to fill in new generateExecutor * make modifyFunctionality dependent on generateExecutor Move logic to new & desired function. Moving step by step. * Remove BashWrapper.inputs It has been unused for 3 years and somewhat complicated the code. * Make DebugPlatform an Executor as well so it gets the same interface * Remove modifyFunctionality methods and use generateExecutor method Add a temporary Executor.get() method to get an executor from a platform * Get executor during readConfig Centralizes logic and is the desired functionality anyway * Create AppliedConfig case class Replaces tuple with Config, Option[Executor], Option[Platform] Already makes code more user friendly but will be more so if e.g. some matches are rewritten Will also ease replacing Option[Platform] with List[Container] * refactor passing through appliedConfig as case class More to do * Refactor to pass AppliedConfig more instead of config & executor & platform Add some more lenses to clean up the code * Convert handleSingleConfigDependency to AppliedConfig Piggy back some minor improvements & documentation * Add BuildStatus in AppliedConfig * remove unused helper function * rename static helper functions * add initial implementation for dockercontainer * add wip implementation of executable executor * Push AppliedConfig down all the way down until Config.readConfig Further reduces logic paths Provide implicit conversion from Config to AppliedConfig Piggy back some more Console.xxx.prints * Improve printing of messages only when there is something to print. This reduces printing of empty lines during a `ns build`, while still using the new logger methods instead of resorting to `Console.(out|err).print` * Add executors & containers in Viash Config Add encoders & decoders Container decoders is currently fixed to DockerContainers since we don't have a `type` field. * Rename container to engine Add `type` field in engine allowing for Circe encoders en decoders to be completed further * rework executables * refactor main, cli, and Viash* classes * strip platforms * Convert platforms to engines and executors Only native & docker so far. Nextflow still to do. Adapted testbenches from `-p x` to `--engine x --executor x` Failed tests from 210 to ~78 * Fix binary || and && operations in ExecutableExecutor * Fix TestAllComponentsSuite `-p x` -> `--engine x --executor x` * change testbenches to use engine & executor instead of platforms * Map NextflowPlatform to NextflowExecutor and optionally NativeEngine * apply engines & executors and remove platforms, allow namespace configs to fail applying executor and engine without failing completely Fix some more testbenches * change testbench checking docker tag names now always appending the image name to the tag * Mark missing variable local before assigning it declaring the variable as local and getting the exit code together erases the exit code * Add a line of comment for the missing variable Otherwise, next iteration of the code might undo the change as it is not obvious why the change is made * Add BuildStatus for when no executor or engine could be applied Previously used BuildError, which is not really true and set error true while now we can differentiate and set error false * Remove test covering no longer available functionality Functionality wasn't used in reality so wasn't converted to engine/executor * Remove testbenches for disabled docker chown Verify that disabling chown throws an error instead of silently ignoring the parameter * Rename executor to runner * Update test temporary folder name * Fix --apply_platform -> --apply_engine & --apply_runner and fix ns exec functionality ns exec didn't work when no engines were allocated * Fix running executables Also fix argument shifts with ---engine and ---setup parameters. whoops. * Tweak viash test cache testbench Add a command to run in the container so we can better check the behaviour * Move DockerPlatform.chown == true assert to DockerPlatform While it's nice to have no implementation in the platforms, moving the assert cleans up things quite a bit. * Convert platforms to runners and engines in the Circe decode step Basically perform pre-processing instead of post-processing Added deprecated and removed annotations Remove `getRunners` & `getEngines` methods, now use `runners` & `engines` directly. * Fix deprecation and removed version tags * Add platforms deprecation warning test * Add a validation step for platforms before converting them to engines and runners We can't auto-infer the type so we have to do it manually instead of calling `validator`. * Ignore StdErr warnings about platforms in some tests tests check either start of the output or a match, which will always give issues if there are some deprecation warnings * Remove more references to platforms and adapt testbash to runners/engines Add basic documentation for engines and runners. Should be extended further. * Remove more references to platforms * move platform subclasses to engines and runners * Update where the DockerPlatform.chown assertion happens Was moved to the validation step, so no need to do it again in the prepare step Improve testbench to check the error output too. * Write a changelog entry. * Strip unused imports from platforms * fix test namespaces and update extra viash underscore components Add -r and -e short arguments for --apply_runner and --apply-engine respectively * Move nextflow resources from io/viash/platforms to io/viash/runners * Fix testbench failing only when running locally and while running sbt test When running only that test locally it was successfully too. Turns out to be a collision in the docker image name. Rewritten the test to use the ConfigDeriver functionality. Lots of the custom code for this test was separately implemented in ConfigDeriver. Piggy back a minor improvement in the config_mods AppendTest testbench. * Add backwards compatibility for `platfroms/...` exports Add testbenches to check legacy paths Add extra testbench to check messing resource error during test * Fix some more references to platforms Somehow these didn't show up earlier * Add runner and engine types in config documentation Fix future reference link in static page * Provide platform backwards compatibility in dependencies We want to support already built components without having them first switch to runners & engines * Use 'executable' as output folder for dependency builds Also provide backwards compatibility with platforms (executable <-> native) when matching runnerIds * Fix dependency example indenting * Add default annotations for platforms, runners, engines This allows skipping the values in the json schema * Create files for local dependency tests from scratch Use minimal config Also run output and check output * Add a remote dependency test Tweak the local dependency a bit too * Add test for dependency with nested dependencies * Add a testbench for config mod paths & filters * Fix dependency code still using console prints directly * Remove unused code Use `orElse` instead * Add parameter validation test for long arguments * WIP merge Nextflow tests fail big time added txt files with snippets of significant sections of code that get removed during the merge * move helper functions from platforms/nextflow to runners/nextflow * merge nextflowplatform changes into nextflowrunner * remove txt * Add changelog entry * Reapply code fixes from develop that didn't get merged properly * Remove WorkflowHelper.nf from git and add it into the .gitignore * Allow dependency resolution to use components that were disabled by a query When rebuilding a pipeline with previously built components, one wants to use `-q`, but this resulted in missing dependencies. Now pick up DisabledByQuery components for local dependencies too. * differentiate disabled and disabled by query status in readConfigs --------- Co-authored-by: Robrecht Cannoodt <[email protected]>
* Basic implementation of splitting platforms into executors and containers For now: - all docker platforms are containers - all native and nextflow platforms are executors the debug platform is just a platform should we add a docker executor? * define Executor and Container as a standalone trait * look for containerDirective in NextflowPlatform using getContainers * add id field to executor * add executorresources * add placeholder objects * Provide temporary wrapper to fill in new generateExecutor * make modifyFunctionality dependent on generateExecutor Move logic to new & desired function. Moving step by step. * Remove BashWrapper.inputs It has been unused for 3 years and somewhat complicated the code. * Make DebugPlatform an Executor as well so it gets the same interface * Remove modifyFunctionality methods and use generateExecutor method Add a temporary Executor.get() method to get an executor from a platform * Get executor during readConfig Centralizes logic and is the desired functionality anyway * Create AppliedConfig case class Replaces tuple with Config, Option[Executor], Option[Platform] Already makes code more user friendly but will be more so if e.g. some matches are rewritten Will also ease replacing Option[Platform] with List[Container] * refactor passing through appliedConfig as case class More to do * Refactor to pass AppliedConfig more instead of config & executor & platform Add some more lenses to clean up the code * Convert handleSingleConfigDependency to AppliedConfig Piggy back some minor improvements & documentation * Add BuildStatus in AppliedConfig * remove unused helper function * rename static helper functions * add initial implementation for dockercontainer * add wip implementation of executable executor * Push AppliedConfig down all the way down until Config.readConfig Further reduces logic paths Provide implicit conversion from Config to AppliedConfig Piggy back some more Console.xxx.prints * Improve printing of messages only when there is something to print. This reduces printing of empty lines during a `ns build`, while still using the new logger methods instead of resorting to `Console.(out|err).print` * Add executors & containers in Viash Config Add encoders & decoders Container decoders is currently fixed to DockerContainers since we don't have a `type` field. * Rename container to engine Add `type` field in engine allowing for Circe encoders en decoders to be completed further * rework executables * refactor main, cli, and Viash* classes * strip platforms * Convert platforms to engines and executors Only native & docker so far. Nextflow still to do. Adapted testbenches from `-p x` to `--engine x --executor x` Failed tests from 210 to ~78 * Fix binary || and && operations in ExecutableExecutor * Fix TestAllComponentsSuite `-p x` -> `--engine x --executor x` * change testbenches to use engine & executor instead of platforms * Map NextflowPlatform to NextflowExecutor and optionally NativeEngine * apply engines & executors and remove platforms, allow namespace configs to fail applying executor and engine without failing completely Fix some more testbenches * change testbench checking docker tag names now always appending the image name to the tag * Mark missing variable local before assigning it declaring the variable as local and getting the exit code together erases the exit code * Add a line of comment for the missing variable Otherwise, next iteration of the code might undo the change as it is not obvious why the change is made * Add BuildStatus for when no executor or engine could be applied Previously used BuildError, which is not really true and set error true while now we can differentiate and set error false * Remove test covering no longer available functionality Functionality wasn't used in reality so wasn't converted to engine/executor * Remove testbenches for disabled docker chown Verify that disabling chown throws an error instead of silently ignoring the parameter * Rename executor to runner * Update test temporary folder name * Fix --apply_platform -> --apply_engine & --apply_runner and fix ns exec functionality ns exec didn't work when no engines were allocated * Fix running executables Also fix argument shifts with ---engine and ---setup parameters. whoops. * Tweak viash test cache testbench Add a command to run in the container so we can better check the behaviour * Move DockerPlatform.chown == true assert to DockerPlatform While it's nice to have no implementation in the platforms, moving the assert cleans up things quite a bit. * Convert platforms to runners and engines in the Circe decode step Basically perform pre-processing instead of post-processing Added deprecated and removed annotations Remove `getRunners` & `getEngines` methods, now use `runners` & `engines` directly. * Fix deprecation and removed version tags * Add platforms deprecation warning test * Add a validation step for platforms before converting them to engines and runners We can't auto-infer the type so we have to do it manually instead of calling `validator`. * Ignore StdErr warnings about platforms in some tests tests check either start of the output or a match, which will always give issues if there are some deprecation warnings * Remove more references to platforms and adapt testbash to runners/engines Add basic documentation for engines and runners. Should be extended further. * Remove more references to platforms * move platform subclasses to engines and runners * Update where the DockerPlatform.chown assertion happens Was moved to the validation step, so no need to do it again in the prepare step Improve testbench to check the error output too. * Write a changelog entry. * Strip unused imports from platforms * fix test namespaces and update extra viash underscore components Add -r and -e short arguments for --apply_runner and --apply-engine respectively * Move nextflow resources from io/viash/platforms to io/viash/runners * Fix testbench failing only when running locally and while running sbt test When running only that test locally it was successfully too. Turns out to be a collision in the docker image name. Rewritten the test to use the ConfigDeriver functionality. Lots of the custom code for this test was separately implemented in ConfigDeriver. Piggy back a minor improvement in the config_mods AppendTest testbench. * Add backwards compatibility for `platfroms/...` exports Add testbenches to check legacy paths Add extra testbench to check messing resource error during test * Fix some more references to platforms Somehow these didn't show up earlier * Add runner and engine types in config documentation Fix future reference link in static page * Provide platform backwards compatibility in dependencies We want to support already built components without having them first switch to runners & engines * Use 'executable' as output folder for dependency builds Also provide backwards compatibility with platforms (executable <-> native) when matching runnerIds * Fix dependency example indenting * Add default annotations for platforms, runners, engines This allows skipping the values in the json schema * Create files for local dependency tests from scratch Use minimal config Also run output and check output * Add a remote dependency test Tweak the local dependency a bit too * Add test for dependency with nested dependencies * Add a testbench for config mod paths & filters * Fix dependency code still using console prints directly * Remove unused code Use `orElse` instead * Add parameter validation test for long arguments * WIP merge Nextflow tests fail big time added txt files with snippets of significant sections of code that get removed during the merge * move helper functions from platforms/nextflow to runners/nextflow * merge nextflowplatform changes into nextflowrunner * remove txt * Add changelog entry * Reapply code fixes from develop that didn't get merged properly * Remove WorkflowHelper.nf from git and add it into the .gitignore * Allow dependency resolution to use components that were disabled by a query When rebuilding a pipeline with previously built components, one wants to use `-q`, but this resulted in missing dependencies. Now pick up DisabledByQuery components for local dependencies too. * differentiate disabled and disabled by query status in readConfigs * Add options to export json schema in strict and minimal versions 'Minimal' removes 'description' fields. 'Strict' removes alternative field definitions. An exception needs to be made for class subtypes, e.g. arguments, otherwise all arguments need to be StringArguments etc. * Fix Json schema export for docker.port Make string arrays the preferred type * Clean up enums in strict mode, move DockerResolveVolume back to platforms * Skip deprecated classes and fields in strict mode Change platform deprecation from 0.8.0 to 0.9.0 * Add exception for NextflowDirectives strict json schema * Don't output `internal functionality` fields during `config view` Same goes for `ns list` Add a custom `deriveConfiguredEncoderStrict` which checks annotations and remove any field annotated with `@internalFunctionality`. Add a `hasInternalFunctionality` and `hasUndocumented` in the `Parameters` class. This means we now always have to output the parameters fields, even if previously skipped, so add an extra filter on the `data` field. Change the `.info` field from `internalFunctionality` to `undocumented` * apply config_mods before applying parent folders to resources applying config_mods uses serialization which now erases parent folder values Adapt some test benches that specifically were testing serialization * Add 'undocumented' values in the json_schema * Use CollectedSchemas.getParameters to perform the reflection This allows for getting annotations from parent classes too Add test that no internal functionality is specified in the config yaml * Restructure deprecation checking code * Add a changelog entry * Use strict encoder for dependencies and repositories * Fix naming of the testResourcesLens * attempt to relax codecov PR thresholds a little bit Allow upto 1% of coverage decrease * Add test checking json decode validator for internal functionality --------- Co-authored-by: Robrecht Cannoodt <[email protected]>
* Test local repositories Test with both absolute and relative paths. * Refactor TestHelper class Extend ExceptionOutput with exitCode and use it in all functions Remove testMain and replace it with testMainWithStdErr functionality and rename it testMain as the sole function Combine functionality of testMainException and testMainException2 * Fix modified testbench checking the wrong output for text * Fix cherry picking * Add changelog entry * Fix merge
* Handle exception when __merge__ uses an invalid yaml Throw a clean exception instead of a stacktrace Handle same issue during dependency resolution for reading config yamls * Add the PR # * Improve exception text when the __merge__ type is invalid Create a new exception type inheriting AbstractConfigException * Add some unit tests
* first draft * Implement remaining functionality and use when reading configs for `ns` * Enter PR #
#569) * Fix github uri -> repo, add local repository and local dependency example * Fix indentation in dependency examples
Do a rough length comparison
undefined
* more vdsl3 testing wip * fix filter * fix filter statement * another permutation of fix filter * fix filter... just kidding, fix assertion * undo comments * remove printlns
Sync develop_0_8 into main develop, fixes changelog and guarantees we have a common history again
Add another minimal test for export json_schema --strict and --minimal
Fix typo spotted by Kai
…uples with more than two elements. (#587) * Add failing example * Implement fix * Add CHANGELOG entry
Fix config decoding not being enforcing strict mode
* Merge arguments into argument_groups in preparse step Fixes #573 * Add changelog entry * Add unit tests covering basic possibilities of arguments and argument_groups * simplify ArgumentGroup verification * Arguments should not be outputted in a strict schema * Throw away the warning when an argument group is specified with strings The functionality was removed in Viash 0.7.0 * Fix severely outdated comment * Use automatic type deriving again for argument groups * Remove test that is no longer valid as the code it tests was removed * Bump the changelog entry to breaking changes
* Bump scala to 2.13.12 and CI java version from 20 to 21 Attempt to run Nextflow with Java 21 too. * Bump the sbt-scroverage version to one that's available for Scala 2.13.12 * Bump sbt to latest version too * disable nextflow tests again with Java version 21 Java 21 support was added to Nextflow in version 23.10.0 NextflowTestHelper.scala forces Nextflow to version 22.04.5 * Add a changelog entry
…onent info field also adding an array of strings in a component info field to store cli config mods Fix some warnings about inference of implicit functions
Add fix for copy pasta mistake in the dependency testbench
* BUG: ownership of output files when using 'docker' profile in nextflow * Add PR number [ci skip]
This reverts commit bed9aa4.
* relativize config paths and derivatives when a package config is present todo relativize towards working directory when no package config is found * relativize paths using the src or target path when no package path is found output & abs-output is still semi-broken in edge cases * push the working directory into namespace exec and use that to relativize the paths in some edge cases * refactor old long regex to chunked version for readability * change test for relative path to test loosely in diverse environments * add changelog entry
#743) * Add label & summary fields for Config, PackageConfig, argument groups, and all argument types * Update CHANGELOG.md * Apply suggestions from code review Co-authored-by: Robrecht Cannoodt <[email protected]> * Update src/main/scala/io/viash/packageConfig/PackageConfig.scala * Apply suggestions from code review --------- Co-authored-by: Robrecht Cannoodt <[email protected]>
* fix findstates * Update src/main/resources/io/viash/runners/nextflow/states/findStates.nf Co-authored-by: Hendrik Cannoodt <[email protected]> * Update src/main/resources/io/viash/runners/nextflow/states/findStates.nf Co-authored-by: Hendrik Cannoodt <[email protected]> --------- Co-authored-by: Hendrik Cannoodt <[email protected]>
#744) * Allow ns query with a config trailing argument to filter which component gets picked up * Add a short test * Update CHANGELOG.md Co-authored-by: Robrecht Cannoodt <[email protected]> --------- Co-authored-by: Robrecht Cannoodt <[email protected]>
* add ---docker_image_id flag * update changelog * fix missing elif --------- Co-authored-by: Hendrik Cannoodt <[email protected]>
* make docker runargs configurable * use argument instead of variable * add entry to changelog * Update src/main/scala/io/viash/runners/ExecutableRunner.scala Co-authored-by: Hendrik Cannoodt <[email protected]> * Update src/main/scala/io/viash/runners/ExecutableRunner.scala --------- Co-authored-by: Hendrik Cannoodt <[email protected]>
* `ExecutableRunner`: Add parameter `docker_automount_prefix` to allow for a custom prefix for automounted folders * add test * update changelog * move environment variable * remove unused variable * remove writing to output (&log) file and instead output to stdout * rename variables for easy copy pasting ^^ * add since tag Co-authored-by: Hendrik Cannoodt <[email protected]> * fix since tag * Apply suggestions from code review Co-authored-by: Robrecht Cannoodt <[email protected]> --------- Co-authored-by: Hendrik Cannoodt <[email protected]>
#754) * Always use local variables when storing the options to prevent overriding in sub functions * Add changelog entry * make all variables in functions local. Only checked bashwrapper * Add test * deduplicate docker test helper functions code was becoming duplicated too much, moved to TestHelper.scala * fix wrong local variable and merge erroneous double space * Update src/main/resources/io/viash/helpers/bashutils/ViashDockerFuns.sh Co-authored-by: Robrecht Cannoodt <[email protected]> --------- Co-authored-by: Robrecht Cannoodt <[email protected]>
some syntax cleanup removed some unused imports fixed some indenting change Scallop syntax for subcommands escape 'export' in cli code
Co-authored-by: Robrecht Cannoodt <[email protected]>
* Create a testbench to verify code is currently not working so we can apply a fix and have a working testbench later * Fix code & improve testbench * Update CHANGELOG.md * Change implementation to use zipWithIndex
* update scala to 2.13.14 * fill in PR# * bump sbt-scoverage to 2.1.1 * rewind sbt-scoverage to 2.1.0
* only fixes/tested `${id}` as an output file parameter
as input file parameter it's broken in a different way
* attempt an alternative strategy for escaping
set variable names using `x='abc'` instead of `x="abc"` which means we only have to escape the `'`.
Not that it fixes *that* much though.
Fix input parameter escaping too
* replace ${id} and ${key} the same way $id and $key are replaced
* Apply suggested changes from @rcannood but only slightly correcter
* Add changelog entry
* Add a short testbench
* Copy package_config in the testing config * add changelog entry
#768) * Fix json schema repositories with name adding the withname in the type * Add a basic schema validation component and use it to verify testbash This test can be extended further later * Add DockerTest annotation to new tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Describe your changes
Merged RC release notes into one
Related issue(s)
Closes #xxxx
Type of Change
Checklist
Requirements:
Tests:
Documentation:
Test Environment