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

Add --extra-args option #39

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Add --extra-args option #39

merged 4 commits into from
Oct 11, 2024

Conversation

cl3to
Copy link
Collaborator

@cl3to cl3to commented Sep 4, 2024

Users can now pass additional arguments in the CLI. These arguments can be used within the command template in the spinner's YAML configuration. The --extra-args parameter accepts a list of key=value pairs, separated by semicolons.

Exemple:

host_list=$(scontrol show hostname "$SLURM_JOB_NODELIST" | tr '\n' ',' | sed 's/,$//')
images_dir="/images"
spinner -c mpich.yaml -r --extra-args 'hosts=$host_list;images=$images_dir'

Closes #20

@rodrigo-ceccato
Copy link
Contributor

LG, only two comments:

  1. Please update the example in: docs/slurm.MD
  2. It would be cool to have it in the CI (and in /docs/examples)
metadata:
  description: Sleep check
  version: "1.0"
  runs: 1
  timeout: 5
  retry: True
  retry_limit: 0

  sleep_test:
    command:
      template: >
        sleep {{sleep_ammount + (extra_time | int)}}

sleep_test:
  sleep_ammount:
    - 1
    - 2

that could be ran with:
spinner -r -c extra_args.yaml --extra-args=extra_time="4"

Overall, I like this new CLI a lot

@rodrigo-ceccato rodrigo-ceccato mentioned this pull request Sep 5, 2024
9 tasks
@cl3to
Copy link
Collaborator Author

cl3to commented Oct 9, 2024

@rodrigo-ceccato,

I've updated the PR based on your suggestions.

One thing to note: the extra arguments are currently only included in the metadata, not in the final dataframe. We can address this later when we revise the YAML template and other related elements.

@rodrigo-ceccato
Copy link
Contributor

LGTM!

Metadata from CLI not being propagated to the Dataframe is a problem IMO, because our model is simpler if we can treat both executions as the same thing (i.e. it behaves the same if the data come from --extra-args or if comes from the YAML fields).

But I do agree that it would be better to fix thsi after PR#40 merges.

What do you think @leiteg ?

@cl3to cl3to requested a review from leiteg October 9, 2024 20:30
@rodrigo-ceccato rodrigo-ceccato merged commit 8397c37 into main Oct 11, 2024
2 checks passed
@rodrigo-ceccato rodrigo-ceccato deleted the feat-extra-args branch October 11, 2024 15:47
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.

Allow extra arguments on the command line invocation
2 participants