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

Add exists module for checking file existence #35572

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@emonty
Contributor

emonty commented Jan 31, 2018

SUMMARY

It's a very common thing to want to do in playbooks to check if a file
exists or not. While the stat module can be used for this, it does a
bunch of checksum calculating and other stuff. While it can be turned
off, it winds up leaving people with playbooks that look like this:

  - name: Check for file
    stat:
       path: ".stestr/failing"
       get_checksum: false
       get_mime: false
       get_md5: false
     register: stestr_stat

  - name: Run command
    command: "{{ stestr_command }} last"
    when: stestr_stat.stat.exists

when they could just be this:

  - name: Check for stestr directory
    exists:
      path: "{{ zuul_work_dir }}/.stestr/failing"
    register: stestr_exists

  - name: Run command
    command: "{{ stestr_command }} last"
    when: stestr_exists.exists
ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

exists

ANSIBLE VERSION

2.4.3

ADDITIONAL INFORMATION

This would make @omgjlk happy. That should be justification enough.

@omgjlk

I like the intent, but there is some work to be done yet here.

lib/ansible/modules/files/exists.py Outdated
@@ -0,0 +1,120 @@
#!/usr/bin/python
# Copyright: (c) 2017, Ansible Project

This comment has been minimized.

@omgjlk

omgjlk Jan 31, 2018

Member

It is now 2018....

lib/ansible/modules/files/exists.py Outdated
DOCUMENTATION = r'''
---
module: stat

This comment has been minimized.

@omgjlk

omgjlk Jan 31, 2018

Member

s/stat/exists/

type: bool
default: 'no'
notes:
- For Windows targets, use the M(win_stat) module instead.

This comment has been minimized.

@omgjlk

omgjlk Jan 31, 2018

Member

This note probably no longer applies, unless you'll be making a Windows version. Since this module will not work on Windows, it should be noted. (at least I think it won't? All the windows modules use powershell so ¯\(ツ)

This comment has been minimized.

@emonty

emonty Jan 31, 2018

Contributor

Yah - I don't really expect to make a windows copy. I think the comment still applies though - windows users need to use win_stat if they want to check file existence. Should we reword it?

This comment has been minimized.

@omgjlk

omgjlk Jan 31, 2018

Member

Hrm. Yeah the comment does still apply, you're right there. A slight tweaking of the wording could be done, or not. Either way that won't block the review.

lib/ansible/modules/files/exists.py Outdated
register: foo_exists
- fail:
msg: "Whoops! /etc/foo.conf does not exist"
when: foo_exists.exists

This comment has been minimized.

@omgjlk

omgjlk Jan 31, 2018

Member

Um, this will fail IF the file exists, which doesn't match the message.

lib/ansible/modules/files/exists.py Outdated
short_description: Check to see if a file or directory exists
description:
- Checks to see if a file exists, similar to stat, but without calculating
extra checksums.

This comment has been minimized.

@omgjlk

omgjlk Jan 31, 2018

Member

Should note about is_dir and is_link here.

Add exists module for checking file existence
It's a very common thing to want to do in playbooks to check if a file
exists or not. While the stat module can be used for this, it does a
bunch of checksum calculating and other stuff. While it can be turned
off, it winds up leaving people with playbooks that look like this:

  - name: Check for file
    stat:
       path: ".stestr/failing"
       get_checksum: false
       get_mime: false
       get_md5: false
     register: stestr_stat

  - name: Run command
    command: "{{ stestr_command }} last"
    when: stestr_stat.stat.exists

when they could just be this:

  - name: Check for stestr directory
    exists:
      path: "{{ zuul_work_dir }}/.stestr/failing"
    register: stestr_exists

  - name: Run command
    command: "{{ stestr_command }} last"
    when: stestr_exists.exists

@emonty emonty force-pushed the emonty:exists-module branch to 6b4936c Jan 31, 2018

@ansibot ansibot added the test label Jan 31, 2018

@omgjlk

One last open question!

@@ -0,0 +1,111 @@
# test code for the exists module
# (c) 2014, James Tanner <tanner.jc@gmail.com>

This comment has been minimized.

@omgjlk

omgjlk Jan 31, 2018

Member

Is Date / Author correct here? I assume this is some % copy pasta, and some % new code.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jan 31, 2018

The test ansible-test sanity --test pylint [?] failed with the following errors:

lib/ansible/modules/files/exists.py:101:25: undefined-variable Undefined variable 'b_path'
lib/ansible/modules/files/exists.py:103:26: undefined-variable Undefined variable 'b_path'

click here for bot help

@sivel

This comment has been minimized.

Member

sivel commented Jan 31, 2018

I've had a brief discussion with other members of the core team in regards to this. We would recommend a solution that "fixes" the stat module, instead of introducing another module.

A new arg could be introduced to turn off all checksumming, to reduce the need for disabling 3 arguments.

As a new module, we won't be accepting this.

@sivel sivel removed the needs_triage label Jan 31, 2018

@mattclay mattclay added the ci_verified label Feb 1, 2018

@ansibot ansibot added the stale_ci label Feb 9, 2018

@jctanner jctanner referenced this pull request Mar 13, 2018

Closed

reviews missing commit id #881

@ansibot ansibot added the affects_2.6 label May 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment