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.

Sign up for GitHub

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

Jump to bottom

win_psexec: Enable -nobanner option by default #23422

Merged
merged 2 commits into from
May 10, 2017
Merged

win_psexec: Enable -nobanner option by default #23422

merged 2 commits into from
May 10, 2017

Conversation

dagwieers
Copy link
Contributor

@dagwieers dagwieers commented Apr 7, 2017 •
edited

SUMMARY

PR also includes:

  • Option nobanner: to influence this flag
  • Improvements to Examples
  • Improvement to Return Value
  • Add types to all parameters now that we have those
  • Add limited integration tests
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

win_psexec

ANSIBLE VERSION

v2.3

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 community_review In order to be merged, this PR must follow the community review workflow. feature_pull_request module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. owner_pr This PR is made by the module's maintainer. windows Windows community labels Apr 7, 2017
@ansibot
Copy link
Contributor

ansibot commented Apr 7, 2017

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/windows/win_psexec.py:0:0: E309 version_added for new option (nobanner) should be 2.4. Currently 0.0

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Apr 7, 2017
@ansibot ansibot added test_pull_requests and removed ci_verified Changes made in this PR are causing tests to fail. labels Apr 7, 2017
@ansibot
Copy link
Contributor

ansibot commented Apr 7, 2017

The test ansible-test sanity --test ansible-doc --python 3.5 failed with the following error:

Command "ansible-doc win_psexec" returned exit status 1.
>>> Standard Error
[ERROR]: unable to parse
/root/src/github.com/ansible/ansible/lib/ansible/modules/windows/win_psexec.py
ERROR! module win_psexec missing documentation (or could not parse documentation): Parsing produced an empty object.

The test ansible-test sanity --test ansible-doc --python 2.7 failed with the following error:

Command "ansible-doc win_psexec" returned exit status 1.
>>> Standard Error
[ERROR]: unable to parse
/root/src/github.com/ansible/ansible/lib/ansible/modules/windows/win_psexec.py
ERROR! module win_psexec missing documentation (or could not parse documentation): Parsing produced an empty object.

The test ansible-test sanity --test ansible-doc --python 2.6 failed with the following error:

Command "ansible-doc win_psexec" returned exit status 1.
>>> Standard Error
[ERROR]: unable to parse
/root/src/github.com/ansible/ansible/lib/ansible/modules/windows/win_psexec.py
ERROR! module win_psexec missing documentation (or could not parse documentation): Parsing produced an empty object.

The test ansible-test sanity --test ansible-doc --python 3.6 failed with the following error:

Command "ansible-doc win_psexec" returned exit status 1.
>>> Standard Error
[ERROR]: unable to parse
/root/src/github.com/ansible/ansible/lib/ansible/modules/windows/win_psexec.py
ERROR! module win_psexec missing documentation (or could not parse documentation): Parsing produced an empty object.

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/windows/win_psexec.py:0:0: E302 DOCUMENTATION is not valid YAML. Line 111 column 1

click here for bot help

@dagwieers dagwieers force-pushed the win_psexec-nobanner branch 9 times, most recently from 855034a to 4a308f6 Compare April 8, 2017 09:03
@nitzmahone nitzmahone removed the needs_triage Needs a first human triage before being processed. label Apr 8, 2017
@dagwieers dagwieers force-pushed the win_psexec-nobanner branch 4 times, most recently from 33b0a44 to b12bb7d Compare April 8, 2017 11:26
@ansibot
Copy link
Contributor

ansibot commented Apr 8, 2017

The test ansible-test sanity --test yamllint failed with the following error:

test/integration/targets/win_psexec/tasks/main.yml:44:81: syntax error: found unknown escape character '{'

click here for bot help

@dagwieers dagwieers force-pushed the win_psexec-nobanner branch 2 times, most recently from 80c4683 to a8825d1 Compare April 8, 2017 18:31
@dagwieers
win_psexec: Enable -nobanner option by default
73c3b26 
PR also includes:
- Option `nobanner:` to remove this flag
- Improvements to Examples
- Improvement to Return Value
- Add types to all parameters now
- Add (limited) integration tests
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 10, 2017
@dagwieers dagwieers mentioned this pull request May 8, 2017
Closed
nitzmahone
nitzmahone requested changes May 9, 2017
View reviewed changes
test/integration/targets/win_psexec/tasks/main.yml Outdated

- name: Create sysinternals directory
win_file:
path: C:\Program Files\sysinternals
Copy link
Member

Choose a reason for hiding this comment

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

Why not unzip/run this in the tempdir in case someone actually has a C:\Program Files\sysinternals dir that wasn't created by the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I just always do it that way :-) Doing it in tempdir also would require me to create a directory, as the zip files is a blob of binaries with no directory. Will fix !

Actually I don't understand why Microsoft doesn't properly package it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed !

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed community_review In order to be merged, this PR must follow the community review workflow. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels May 9, 2017
@dagwieers dagwieers force-pushed the win_psexec-nobanner branch 8 times, most recently from 6870327 to cc3a927 Compare May 9, 2017 11:05
@nitzmahone nitzmahone merged commit e855274 into ansible:devel May 10, 2017
KKoukiou pushed a commit to KKoukiou/ansible that referenced this pull request May 22, 2017
@dagwieers @KKoukiou
win_psexec: Enable -nobanner option by default (ansible#23422)
4ab5392 
* win_psexec: Enable -nobanner option by default

PR also includes:
- Option `nobanner:` to remove this flag
- Improvements to Examples
- Improvement to Return Value
- Add types to all parameters now
- Add (limited) integration tests

* Move sysinternals into C:\Windows\Temp
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nitzmahone nitzmahone nitzmahone approved these changes

Assignees
No one assigned
Labels
affects_2.4 This issue/PR affects Ansible v2.4 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. owner_pr This PR is made by the module's maintainer. windows Windows community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dagwieers @ansibot @nitzmahone