Skip to content

fixed bad copy paste errors according to the issue https://github.com…#68988

Open
arinochk wants to merge 1 commit into
saltstack:masterfrom
arinochk:fixes-68987
Open

fixed bad copy paste errors according to the issue https://github.com…#68988
arinochk wants to merge 1 commit into
saltstack:masterfrom
arinochk:fixes-68987

Conversation

@arinochk
Copy link
Copy Markdown

What does this PR do?

What issues does this PR fix or reference?

Fixes two separate issues:

  1. In salt/modules/win_dacl.py, replaces an incorrect variable check inside a condition. The code originally checked permission but then used acetype for transformation, which looks like a copy-paste mistake. Now acetype is properly normalised to uppercase (or set to None if falsy).
  2. In salt/modules/schedule.py, corrects an error log message that referenced the wrong variable. The exception occurs when parsing current_time, but the log erroneously printed time_fmt twice. The log now correctly outputs the problematic current_time value along with the format string.

What issues does this PR fix or reference?

Fixes #68967

Previous Behavior

  1. win_dacl.py: the code did if permission: but then used acetype.upper(), leading to possible AttributeError if acetype was None, and generally inconsistent behaviour.
  2. schedule.py: when a date string could not be parsed, the error log showed time_fmt twice instead of showing the actual unparsable string (current_time), making debugging harder.

New Behavior

  1. win_dacl.py: the condition now checks if acetype: and then assigns acetype = acetype.upper(). If acetype is falsy, it becomes None.
  2. schedule.py: the error log now outputs current_time (the string that failed to parse) and time_fmt (the format used), e.g. log.error("Date string could not be parsed: %s, %s", current_time, time_fmt).

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

@bdrx312
Copy link
Copy Markdown
Contributor

bdrx312 commented Apr 23, 2026

Thanks for the report and the fix. You will need to add a change log. See https://docs.saltproject.io/en/latest/topics/development/contributing.html. The maintainers now also require adding test cases to the automated pytests; so, I am sure they will request that you add some tests before they approve this.

@arinochk
Copy link
Copy Markdown
Author

Hi! Unfortunately, I won't be able to write the tests at the moment, as I don't know how you write them. I'd appreciate it if someone could help with that.

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.

Static analyzer detected a potential bad copy paste

2 participants