Skip to content
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

Arbitrary CLI args for shell tasks #66

Merged
merged 20 commits into from
Dec 17, 2024
Merged

Arbitrary CLI args for shell tasks #66

merged 20 commits into from
Dec 17, 2024

Conversation

GeigerJ2
Copy link
Collaborator

@GeigerJ2 GeigerJ2 commented Dec 16, 2024

This PR adds the _CliArgsBaseModel class (inspired from _WhenBaseModel), which replaces the command_option and input_arg_options of the ConfigShellTaskSpecs. Validation is applied on the correctness of the keyword and positional arguments to ensure they start with - or --.

I updated the three test YAML files accordingly. This change led to changes in the pretty-print test text files, which is also part of the PR. @ all: Currently, they all appear on one line. Not sure if formatting should be done nicer here. I saw that the self.as_block is only applied for entities of the ConfigCycleTask, so I just left the .txt files as I got them from running the test and updated them in the PR.

Actually making something useful out of these arguments only happens when the WG is created, so will require #45 to be merged.

GeigerJ2 and others added 11 commits December 3, 2024 16:29
The parameters specified in the yaml corresponding to specific task
plugins file are now passed to their corresponding classes in core.
ShellTask and IconTask. For that pydantic discrimantors are used.
See for more information
https://docs.pydantic.dev/latest/concepts/unions/#discriminated-unions-with-callable-discriminator

To test the new changes formatting of represented member variables has
been added to the formatting of `core.Task` objects.

Documentation to the `core.Plugin` has been added and the class has been
renamed to `core.TaskPlugin`.
@GeigerJ2 GeigerJ2 force-pushed the feature/49/shell-args branch from 55cb807 to 3b87663 Compare December 16, 2024 11:57
@GeigerJ2 GeigerJ2 force-pushed the feature/49/shell-args branch from c472563 to 9f52a76 Compare December 16, 2024 14:06
@GeigerJ2 GeigerJ2 changed the title WIP: Arbitrary CLI args for shell tasks Arbitrary CLI args for shell tasks Dec 16, 2024
@GeigerJ2 GeigerJ2 marked this pull request as ready for review December 16, 2024 14:17
src/sirocco/parsing/_yaml_data_models.py Outdated Show resolved Hide resolved
src/sirocco/parsing/_yaml_data_models.py Show resolved Hide resolved
src/sirocco/parsing/_yaml_data_models.py Show resolved Hide resolved
tests/files/configs/test_config_large.yml Outdated Show resolved Hide resolved
@leclairm
Copy link
Contributor

I updated the three test YAML files accordingly. This change led to changes in the pretty-print test text files, which is also part of the PR. @ALL: Currently, they all appear on one line. Not sure if formatting should be done nicer here. I saw that the self.as_block is only applied for entities of the ConfigCycleTask, so I just left the .txt files as I got them from running the test and updated them in the PR.

At some point, we might have to differentiate between pretty printing for users and for serialized test data. For now, I'd say we keep it his way as testing is more important. Later we could work on stg like a verbosity level and other pretty printing options for users as opposed to testing.

@GeigerJ2
Copy link
Collaborator Author

Thanks for the quick review, @leclairm! I addressed your points. For the idea of specifying flags as keywords without values, maybe we can keep that in mind for the future, and advance as is for now? I'd like to continue with the WorkGraph creation this afternoon, so that these CLI arguments actually get resolved to something meaningful. So it would be nice to get this merged before.

At some point, we might have to differentiate between pretty printing for users and for serialized test data. For now, I'd say we keep it his way as testing is more important. Later we could work on stg like a verbosity level and other pretty printing options for users as opposed to testing.

Fully agree!

@leclairm
Copy link
Contributor

For the idea of specifying flags as keywords without values, maybe we can keep that in mind for the future, and advance as is for now? I'd like to continue with the WorkGraph creation this afternoon, so that these CLI arguments actually get resolved to something meaningful. So it would be nice to get this merged before.

All good then

Copy link
Contributor

@leclairm leclairm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all good, keep merging kw args and flags for later

@GeigerJ2 GeigerJ2 merged commit 3520519 into main Dec 17, 2024
3 checks passed
@GeigerJ2 GeigerJ2 deleted the feature/49/shell-args branch December 17, 2024 12:53
GeigerJ2 added a commit that referenced this pull request Dec 17, 2024
This PR adds the `_CliArgsBaseModel` class (inspired from `_WhenBaseModel`), which replaces the `command_option` and `input_arg_options` of the `ConfigShellTaskSpecs`. Validation is applied on the correctness of the keyword and positional arguments to ensure they start with `-` or `--`.

The three test YAML files are updated accordingly, leading to changes in the pretty-print test text files, which is also part of the PR.

Actually making something useful out of these arguments only happens when the WG is created, so will require #45 to be merged.
agoscinski added a commit that referenced this pull request Dec 30, 2024
TODO remove expandvars hack, use by default local folder

TODO remove example reference from tests

Arbitrary CLI args for shell tasks (#66)

This PR adds the `_CliArgsBaseModel` class (inspired from `_WhenBaseModel`), which replaces the `command_option` and `input_arg_options` of the `ConfigShellTaskSpecs`. Validation is applied on the correctness of the keyword and positional arguments to ensure they start with `-` or `--`.

The three test YAML files are updated accordingly, leading to changes in the pretty-print test text files, which is also part of the PR.

Actually making something useful out of these arguments only happens when the WG is created, so will require #45 to be merged.

Fix svgs being created in main directory. (#67)

Create `svg` files in subdirectory under `tests/files/svgs` instead of top-level directory, and ignore them via `.gitignore`, while syncing the `svgs` directory.
Merge remote-tracking branch 'origin/main' into workgraph

---

Pass single argument string to `ShellTasks` (#72)

* Try to make current example YAML files run through.

* Update class names after rebase/merge

* Add `no_icon` config file

* First working version apart from flags.

* Try fixing argument format.

* Current state before branch-off.

* Pass one multi-line string as `cli_argument`

* Cleanup.

* Pass arguments as list.

* Remove `workgraph-dev.py` dev file.

* Fix issues from `hatch fmt`

reimplementing cli_arguments

change to unspecified inputs are added as positional cli args automatically

delete examples

add files required to run the workflow

remove old files

use rootdir from config

move files to confi folder

adapt configs to relativ to config folder

rm expandvars

fix hatch fmt

fix hatch fmt

update reference

rm comment
agoscinski added a commit that referenced this pull request Dec 30, 2024
TODO remove expandvars hack, use by default local folder

TODO remove example reference from tests

Arbitrary CLI args for shell tasks (#66)

This PR adds the `_CliArgsBaseModel` class (inspired from `_WhenBaseModel`), which replaces the `command_option` and `input_arg_options` of the `ConfigShellTaskSpecs`. Validation is applied on the correctness of the keyword and positional arguments to ensure they start with `-` or `--`.

The three test YAML files are updated accordingly, leading to changes in the pretty-print test text files, which is also part of the PR.

Actually making something useful out of these arguments only happens when the WG is created, so will require #45 to be merged.

Fix svgs being created in main directory. (#67)

Create `svg` files in subdirectory under `tests/files/svgs` instead of top-level directory, and ignore them via `.gitignore`, while syncing the `svgs` directory.
Merge remote-tracking branch 'origin/main' into workgraph

---

Pass single argument string to `ShellTasks` (#72)

* Try to make current example YAML files run through.

* Update class names after rebase/merge

* Add `no_icon` config file

* First working version apart from flags.

* Try fixing argument format.

* Current state before branch-off.

* Pass one multi-line string as `cli_argument`

* Cleanup.

* Pass arguments as list.

* Remove `workgraph-dev.py` dev file.

* Fix issues from `hatch fmt`

reimplementing cli_arguments

change to unspecified inputs are added as positional cli args automatically

delete examples

add files required to run the workflow

remove old files

use rootdir from config

move files to confi folder

adapt configs to relativ to config folder

rm expandvars

fix hatch fmt

fix hatch fmt

update reference

rm comment
agoscinski added a commit that referenced this pull request Dec 31, 2024
TODO remove expandvars hack, use by default local folder

TODO remove example reference from tests

Arbitrary CLI args for shell tasks (#66)

This PR adds the `_CliArgsBaseModel` class (inspired from `_WhenBaseModel`), which replaces the `command_option` and `input_arg_options` of the `ConfigShellTaskSpecs`. Validation is applied on the correctness of the keyword and positional arguments to ensure they start with `-` or `--`.

The three test YAML files are updated accordingly, leading to changes in the pretty-print test text files, which is also part of the PR.

Actually making something useful out of these arguments only happens when the WG is created, so will require #45 to be merged.

Fix svgs being created in main directory. (#67)

Create `svg` files in subdirectory under `tests/files/svgs` instead of top-level directory, and ignore them via `.gitignore`, while syncing the `svgs` directory.
Merge remote-tracking branch 'origin/main' into workgraph

---

Pass single argument string to `ShellTasks` (#72)

* Try to make current example YAML files run through.

* Update class names after rebase/merge

* Add `no_icon` config file

* First working version apart from flags.

* Try fixing argument format.

* Current state before branch-off.

* Pass one multi-line string as `cli_argument`

* Cleanup.

* Pass arguments as list.

* Remove `workgraph-dev.py` dev file.

* Fix issues from `hatch fmt`

reimplementing cli_arguments

change to unspecified inputs are added as positional cli args automatically

delete examples

add files required to run the workflow

remove old files

use rootdir from config

move files to confi folder

adapt configs to relativ to config folder

rm expandvars

fix hatch fmt

fix hatch fmt

update reference

rm comment

implement wait_on

clean up code

clean up add doc
agoscinski added a commit that referenced this pull request Jan 1, 2025
TODO remove expandvars hack, use by default local folder

TODO remove example reference from tests

Arbitrary CLI args for shell tasks (#66)

This PR adds the `_CliArgsBaseModel` class (inspired from `_WhenBaseModel`), which replaces the `command_option` and `input_arg_options` of the `ConfigShellTaskSpecs`. Validation is applied on the correctness of the keyword and positional arguments to ensure they start with `-` or `--`.

The three test YAML files are updated accordingly, leading to changes in the pretty-print test text files, which is also part of the PR.

Actually making something useful out of these arguments only happens when the WG is created, so will require #45 to be merged.

Fix svgs being created in main directory. (#67)

Create `svg` files in subdirectory under `tests/files/svgs` instead of top-level directory, and ignore them via `.gitignore`, while syncing the `svgs` directory.
Merge remote-tracking branch 'origin/main' into workgraph

---

Pass single argument string to `ShellTasks` (#72)

* Try to make current example YAML files run through.

* Update class names after rebase/merge

* Add `no_icon` config file

* First working version apart from flags.

* Try fixing argument format.

* Current state before branch-off.

* Pass one multi-line string as `cli_argument`

* Cleanup.

* Pass arguments as list.

* Remove `workgraph-dev.py` dev file.

* Fix issues from `hatch fmt`

reimplementing cli_arguments

change to unspecified inputs are added as positional cli args automatically

delete examples

add files required to run the workflow

remove old files

use rootdir from config

move files to confi folder

adapt configs to relativ to config folder

rm expandvars

fix hatch fmt

fix hatch fmt

update reference

rm comment

implement wait_on

clean up code

clean up add doc

cli parameters

update tests

test work

fix everything
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants