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

Refactor win_system.py #25992

Merged
merged 6 commits into from
Aug 5, 2015
Merged

Refactor win_system.py #25992

merged 6 commits into from
Aug 5, 2015

Conversation

twangboy
Copy link
Contributor

@twangboy twangboy commented Aug 4, 2015

Fixes: #12255
The problem was dumping unicode from python to the windows terminal when shelling out using cmd.run. The unicode didn't translate very well from python to the windows code page.
Removed all code that was shelling out.
Transitioned to pywin32 as much as possible.
Used ctype libraries for those unavailable in pywin32.
Improved documentation.
Added shutdown_abort, lock, and unjoin_domain functions.
Added set_system_date_time which is used by set_system_date and set_system_time

Support for reboot, shutdown, etc
'''
from __future__ import absolute_import

# Import python libs
import logging
import re
import datetime
from ctypes import windll
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be gated and will cause import errors on non-Windows sytems.

@cachedout
Copy link
Contributor

Hi @twangboy. This has some lint errors and an import which needs to be gated for cross-platorm compatability. Please let me know when you've got those issues fixed up and we'll get this merged. Thanks.

@cachedout cachedout added Windows Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged Medium Change pending-changes The pull request needs additional changes before it can be merged labels Aug 4, 2015
@jfindlay jfindlay changed the title Refactor win_service.py Refactor win_system.py Aug 5, 2015
cachedout pushed a commit that referenced this pull request Aug 5, 2015
@cachedout cachedout merged commit 200bff7 into saltstack:2015.5 Aug 5, 2015
@twangboy twangboy deleted the fix_12255 branch February 18, 2016 23:12
@ghost
Copy link

ghost commented Jun 2, 2016

@twangboy, I have a question about this pr. I noticed that you used

from salt.modules.reg import read_value

instead of using

__salt__['reg.read_value'](...)

in your code. I was wondering why you did this. This is because I want to make a PR that affects this file and will use other function from the reg module and I want to understand this.

If anyone else can answer that question I would be grateful.

@twangboy
Copy link
Contributor Author

twangboy commented Jun 2, 2016

@hrumph Perhaps because I didn't know what I was doing. Feel free to use __salt__ functions

@ghost
Copy link

ghost commented Jun 2, 2016

Thanks @twangboy. I thought that there might be some esoteric reason that I'd never be able to figure out without someone telling me first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-changes The pull request needs additional changes before it can be merged Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants