Allow specifying --diff for ansible-pull

[debugloop]

SUMMARY

The ansible-pull command does not support the --diff (and --syntax-check) flags. This is due to the fact that the PullCLI parsing does not include the general check_opts CLI flag group which contains the two flags in question, as well as the --check flag. The check_opts group was propably left out due to a conflict between the ansible-pull specific --checkout/-C flag and the --check/-C flag which is part of the default CLI. Instead, the --check flag was added manually to the ansible-pull CLI parser, without its shorthand -C.

This PR does the same for the --diff and --syntax-check flags, and also adds comments describing the rationale behind adding these three manually (and without their shorthand versions).

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

ansible-pull

ANSIBLE VERSION

ansible 2.6.0
  config file = None
  configured module search path = [u'/home/danieln/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/danieln/.virtualenvs/ansible-dev/lib/python2.7/site-packages/ansible-2.6.0-py2.7.egg/ansible
  executable location = /home/danieln/.virtualenvs/ansible-dev/bin/ansible
  python version = 2.7.14 (default, Jan  5 2018, 10:41:29) [GCC 7.2.1 20171224]

ADDITIONAL INFORMATION

The change works as expected. Running ansible-pull --diff -U git@github.com:debugloop/test-playbook.git test.yml on my machine yields the following results. The playbook can be viewed in said repo, but it is just a copy to a tmpfile with static content.

Before:

Usage: ansible-pull -U <repository> [options] [<playbook.yml>]

ansible-pull: error: no such option: --diff

After:

Starting Ansible Pull at 2018-03-20 15:52:06
/home/danieln/.virtualenvs/ansible-dev/bin/ansible-pull --diff -U git@gitlab.com:danieln/test-playbook.git test.yml
 [WARNING]: Could not match supplied host pattern, ignoring: icemark
localhost | SUCCESS => {
    "after": "09151533adff4ec72cac060c03c00bce1bfe8a90", 
    "before": "09151533adff4ec72cac060c03c00bce1bfe8a90", 
    "changed": false, 
    "remote_url_changed": false
}
 [WARNING]: Unable to parse /etc/ansible/hosts as an inventory source
 [WARNING]: No inventory was parsed, only implicit localhost is available
 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match
'all'
 [WARNING]: Could not match supplied host pattern, ignoring: icemark

PLAY [test] ************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************
ok: [localhost]

TASK [test writing a file] *********************************************************************************************
--- before: /tmp/test.txt
+++ after: /home/danieln/.ansible/tmp/ansible-local-155702xmYGP/tmpmP6VBq
@@ -1,3 +1,5 @@
 blabla
+bloblo
 blibli
+bleble
 blublu

changed: [localhost]

PLAY RECAP *************************************************************************************************************
localhost                  : ok=2    changed=1    unreachable=0    failed=0

[webknjaz]

!component =lib/ansible/cli/pull.py

[debugloop]

@webknjaz can you rerun the CI? I don't think the failure has to do with my changes, some test says Connection failure: ('The read operation timed out',) for some aws domain.

[mattclay]

@debugloop I've restarted CI.

[ansibot]

@debugloop this PR contains the following merge commits:

• 919a87c

Please rebase your branch to remove these commits.

click here for bot help

[debugloop]

Sorry to bother you guys, but it's been a month. What can I do to help get this approved? Anything you need from my side?

[bcoca]

im not sure syntax check makes any sense in this context, why do it via pull? the playbook should be checked by ansible-playbook, using pull as a proxy for a check seems contrived.

[debugloop]

It is. This change works around the Check CLI Option Group being blocked by the -C flag being already present in the ansible-pull specifig flags. I was originally going for the --diff option, but ended up including all flags from the check group. I can remove it of course.

[debugloop]

I removed the syntax checking option from this PR and used the opportunity to squash my commits and rebase to latest devel.

[bcoca]

tempted to say just use check_opts=True and remove syntax check afterward with remove_option (we do this in ansible-inventory).

[debugloop]

If we did that, we'd have to remove --check too and and re-add it without it's shorthand -C? Instead of check_opts=False with two adds we'd end up with a True and two removals and one re-add. I suppose it would reduce the number of duplicate help strings by one. You prefer the second option? Then I'll invert the current PR to do it that way.

[bcoca]

if it were not for the -C i would not say 'tempted' but actually request it

[debugloop]

Okay, I'll consider this merge-ready then.

[debugloop]

Hello @dagwieers , sorry for bugging you but this seems to be waiting for your review. It's been a while and I've rebased the PR with the latest devel again. I guess this feature won't make 2.6 now that RCs have been released?

[hExPY]

+1 please merge :)

[dagwieers]

@debugloop That's easily fixed ;-) Although I don't think this was the blocking issue...

[dagwieers]

@bcoca Can this be merged for v2.7 ?

Update: Maybe I'll put it on the core meeting agenda instead.

[bcoca]

!rebuild_merge