Skip to content

Conversation

morrison12
Copy link
Contributor

@morrison12 morrison12 commented Aug 12, 2025

The _chdir may require the _sudo which has been removed from arguments and thus given it isn't need for the rm, we remove the _chdir to avoid it failing due to the lack of the sudo

Intended to fix #1426

  • [ x ] Pull request is based on the default branch (3.x at this time)
  • [ x ] Pull request includes tests for any new/updated operations/facts
  • [ x ] Pull request includes documentation for any new/updated operations/facts
  • [x ] Tests pass (see scripts/dev-test.sh)
  • [ x] Type checking & code style passes (see scripts/dev-lint.sh)

@morrison12 morrison12 changed the title exclude _chdir from global arguments used when removing temporary file exclude _chdir from global arguments used when removing temporary file (fix #1426) Aug 12, 2025
Copy link
Member

@Fizzadar Fizzadar left a comment

Choose a reason for hiding this comment

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

Left an additional comment in the issue to discuss.

rm_arguments = arguments.copy()
# _chdir is the only one of the global arguments that could require _sudo to succeed
# and _sudo isn't present in arguments as removed above
rm_arguments.pop("_chdir", False)
Copy link
Member

Choose a reason for hiding this comment

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

Appears unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was me getting interrupted I think 😒

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.

file.put with _sudo=True and _chdir is not None can fail if the sudo is required for the chdir to succeed
2 participants