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

implement support for running interactive commands with run_shell_cmd #4453

Merged
merged 18 commits into from
Apr 6, 2024

Conversation

boegel
Copy link
Member

@boegel boegel commented Feb 14, 2024

Currently sits on top of #4444, which should definitely get merged first - I'll sync with PR with 5.0.x branch once that's done.

WIP because: (ready for review)

  • more tests for run_cmd_qa should also be implemented for run_shell_cmd using qa_patterns (done)
  • use of qa_wait_patterns not supported yet; (now implemented)
  • probably need to add a qa_timeout option (equivalent to maxhits in run_cmd_qa); (implemented)
  • maybe some refactoring should be done, like moving code to actually answer questions to a dedicated (private) _handle_cmd_qa function, to avoid making run_shell_cmd even bigger than it already is (done, see _answer_question helper function);
  • testing with actual easyblocks not done yet (see use run_shell_cmd in custom easyblock for WRF easybuild-easyblocks#3270)

@boegel boegel added this to the 5.0 milestone Feb 14, 2024
@easybuilders easybuilders deleted a comment from boegelbot Feb 14, 2024
@easybuilders easybuilders deleted a comment from boegelbot Feb 21, 2024
@easybuilders easybuilders deleted a comment from boegelbot Feb 21, 2024
@easybuilders easybuilders deleted a comment from boegelbot Mar 2, 2024
@easybuilders easybuilders deleted a comment from boegelbot Mar 2, 2024
@boegel boegel marked this pull request as ready for review April 3, 2024 16:34
@boegel boegel changed the title implement support for running interactive commands with run_shell_cmd (WIP) implement support for running interactive commands with run_shell_cmd Apr 3, 2024
easybuild/tools/run.py Outdated Show resolved Hide resolved
Comment on lines 283 to 284
:param qa_wait_patterns: list of 2-tuples with patterns for non-questions
and number of iterations to allow these patterns to match with end out command output
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? Specifically the 'and number of iterations to allow these patterns to match with end out command output' part.

Copy link
Member Author

Choose a reason for hiding this comment

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

No it doesn't, but that does hint towards something in run_cmd_qa that some easyblocks may be using but is not supported yet in run_shell_cmd.
I can look into implementing that when we hit it, we should get this PR merged ASAP.

Fixed docstring in 204f328

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like that feature isn't used in the current easyblocks that use the no_qa option of run_cmd_qa, so we can reimplement it if/when the need arises...

@branfosj branfosj enabled auto-merge April 6, 2024 08:25
@branfosj branfosj merged commit 471bbcc into easybuilders:5.0.x Apr 6, 2024
35 checks passed
@boegel boegel deleted the run_shell_cmd_qa branch April 6, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants