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

Apply CLI clean changes to correct list #6011

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Mar 7, 2024

Fixes an otherwise unrecorded bug reported by @matthewrmshin .

To Reproduce

Ensure that Cylc will attempt remote reinvocation (global.cylc[scheduler][run hosts]available is set to >=1 remote host).

cd ~/cylc-src/workflow
cylc vip --workflow-name beeblebrox --no-run-name

Should result in...

$ cylc play   # Cylc VIP listing command with arguments.
....
cylc: error: Wrong number of arguments (too few)

Cause

cleanup_sysargv was modifying sys.argv before over-writing it with a new list of args.

Reviewers should

Aside from usual checks reviewers should check this change against cylc-rose tests: The original report involved a rose option config.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX. Bug fix does not require docs beyond change log.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@wxtim wxtim force-pushed the fix.sys-argv-cleaner branch from 10240aa to 8efc72a Compare March 7, 2024 14:02
@wxtim wxtim self-assigned this Mar 7, 2024
@wxtim wxtim added bug Something is wrong :( small labels Mar 7, 2024
@wxtim wxtim added this to the cylc-8.2.5 milestone Mar 7, 2024
@MetRonnie MetRonnie requested review from MetRonnie and removed request for oliver-sanders March 12, 2024 15:14
@MetRonnie MetRonnie changed the base branch from master to 8.2.x March 12, 2024 15:26
@MetRonnie
Copy link
Member

I've changed the base branch to 8.2.x to match the milestone

changes.d/6011.fix.md Outdated Show resolved Hide resolved
Co-authored-by: Ronnie Dutta <[email protected]>
@wxtim wxtim requested a review from MetRonnie March 13, 2024 09:55
@oliver-sanders oliver-sanders merged commit 2678b53 into cylc:8.2.x Mar 18, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :( small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants