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

[WIP] Fix wrong return value with check mode in get_url module #65703

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open

[WIP] Fix wrong return value with check mode in get_url module #65703

wants to merge 1 commit into from

Conversation

knagamin
Copy link
Contributor

@knagamin knagamin commented Dec 10, 2019

SUMMARY
  • Fixes #65687

  • Unify 'GET' HTTP method in get_url.py

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

get_url

ADDITIONAL INFORMATION
None

@ansibot
Copy link
Contributor

ansibot commented Dec 10, 2019

@ansibot ansibot added WIP affects_2.10 bug module needs_triage net_tools small_patch support:core labels Dec 10, 2019
* Fixes #65687

* Unify HTTP method into GET in get_url.py
@bmillemathias
Copy link
Contributor

bmillemathias commented Dec 10, 2019

Hello,

I don't really have an expertise on tests but I think some integrations code should be updated as starting

- name: test HTTP HEAD request for file in check mode
refers to HEAD test.

Best.

@ansibot ansibot added the stale_ci label Dec 18, 2019
@samdoran samdoran removed the needs_triage label Dec 19, 2019
@kustodian
Copy link
Contributor

kustodian commented Feb 25, 2020

I still don't think this is the proper way to fix issue #65687.

force parameter should also be accounted for if the task will be changed or not. Doing just HEAD in case of check mode is a cool improvement so that the module doesn't download anything unless needed. But, why does the module even fetch anything if force=false? If force=false the module should only check if the dest file exists or not, it doesn't need to fetch anything.

@ansibot ansibot added the needs_repo label Feb 25, 2020
@ansibot ansibot added the needs_rebase label May 16, 2020
@ansibot ansibot added pre_azp and removed stale_ci labels Dec 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 bug has_issue module needs_rebase needs_repo net_tools pre_azp small_patch support:core WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants