Fix docs get_url dest comment. Checksum does not prevent download.

[vbotka]

SUMMARY

Checksum does not prevent download if dest is dir. It's not clear from the documentation.

get_url checksum says:

"Additionally, if a checksum is passed to this parameter, and the file exists under the dest location, the destination_checksum would be calculated, and if checksum equals destination_checksum, the file download would be skipped ..."

get_url dest says:

"If dest is a directory, the file will always be downloaded (regardless of the force option) ..."

The block comparing the checksum is skipped if dest is a directory
https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/get_url.py#L539

if not dest_is_dir and os.path.exists(dest):

ISSUE TYPE

• Docs Pull Request

COMPONENT NAME

get_url

ADDITIONAL INFORMATION

It's easier to put a directory into dest and let get_url complete the path. Quoting from dest comment:

"If dest is a directory, either the server provided filename or, if none provided, the base name of the URL on the remote server will be used."

Unfortunately, in this case, the checksum of the existing file is not compared and the file is downloaded. A simple solution is to document it:

--- a/lib/ansible/modules/get_url.py
+++ b/lib/ansible/modules/get_url.py
@@ -40,7 +40,8 @@ options:
         none provided, the base name of the URL on the remote server will be
         used. If a directory, C(force) has no effect.
       - If C(dest) is a directory, the file will always be downloaded
-        (regardless of the C(force) option), but replaced only if the contents changed..
+        (regardless of the C(force) and C(checksum) option), but
+        replaced only if the contents changed.
     type: path
     required: true
   tmp_dest:

Verified scenario

• dest is a directory
• checksum is present

1. get_url fetch the file
2. get_url repeatedly fetch the file (burning resources) without testing the checksum of the already fetched file first

[bcoca]

doesn't the checksum indicate if the content changed?

[vbotka]

doesn't the checksum indicate if the content changed?

Content of what?

[vbotka]

For the record. The file is unconditionally downloaded but the description of the parameters indicates the possibility of avoiding the download

• checksum: "... if checksum equals destination_checksum, the file download would be skipped ..."
• dest: "... If dest is a directory, the file will always be downloaded ..."
• force: "... If no, the file will only be downloaded if the destination does not exist. Generally should be yes only for small local files. "

It would probably be better to say: "The file will always be downloaded."

Notes:

• Access to dest is tested after download

[samccann]

@bcoca Can you respond here or approve the docs change? thanks.

[vbotka]

@bcoca Can you respond here or approve the docs change? thanks.

The code was updated. Now, it would be easier to test the checksum when dest is a directory before the download.

[s-hertel]

This is accurate. We can't compare the checksum until we have the destination filename. If dest is a directory the url is fetched to determine the filename (which may come from either the headers or the URL).

[samccann]

Thanks @vbotka for the Ansible docs fix!