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

Update uri.py #56395

Merged
merged 8 commits into from May 20, 2019

Conversation

Projects
None yet
4 participants
@JonTheNiceGuy
Copy link
Contributor

commented May 14, 2019

Added two changes.

  1. Note that uri doesn't honor the no_proxy environment variable (due to #52705), and suggest a work around.
  2. Added an example showing a test waiting for a URL to become available (using the until:, retries: and delay: settings) - based on https://gist.github.com/mikeifomin/67e233cd461331de16707ef59a07e372#gistcomment-2718587

label: docsite_pr

SUMMARY

Added documentation based on my own experiences

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

uri

ADDITIONAL INFORMATION
Update uri.py
Added two changes.
1. Note that uri doesn't honor the no_proxy environment variable (due to #52705), and suggest a work around.
2. Added an example showing a test waiting for a URL to become available (using the `until:`, `retries:` and `delay:` settings) - based on https://gist.github.com/mikeifomin/67e233cd461331de16707ef59a07e372#gistcomment-2718587

label: docsite_pr
@acozine

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

@JonTheNiceGuy thanks for suggesting these changes to help Ansible users understand the URI module better. As it is, the PR isn't the best fit for the Ansible documentation style, but we'd be happy to merge content that covers this ground. Can you update the PR to address these concerns?

  1. Not honoring the no_proxy environment variable is a python constraint, not an issue with Ansible or this particular module. Could you present your useful workaround as an example rather than in the notes field?
  2. The pause-this-play example with until: is useful, but at 5 seconds per retry, 10000 retries is more than 12 hours. Could you use a shorter timeframe?

If you have questions or concerns, or want to discuss documentation, feel free to join the #ansible-docs channel on freenode IRC.

@acozine acozine removed the needs_triage label May 14, 2019

@JonTheNiceGuy

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

Hopefully this is more in line with what you're after @acozine ?

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

@JonTheNiceGuy This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels May 14, 2019

Update uri.py
Following comments to the PR

@JonTheNiceGuy JonTheNiceGuy force-pushed the JonTheNiceGuy:patch-3 branch from 9545eaa to 9b15307 May 14, 2019

@JonTheNiceGuy

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

Fixed commit message

@acozine

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

@JonTheNiceGuy thanks for the quick response! Yes, this is what I had in mind. CI failure is a whitespace issue now: ERROR: lib/ansible/modules/net_tools/basics/uri.py:272:1: W293 blank line contains whitespace

@ansibot ansibot added core_review and removed needs_revision labels May 14, 2019

Show resolved Hide resolved lib/ansible/modules/net_tools/basics/uri.py Outdated
Show resolved Hide resolved lib/ansible/modules/net_tools/basics/uri.py Outdated
Show resolved Hide resolved lib/ansible/modules/net_tools/basics/uri.py Outdated

@ansibot ansibot added needs_revision and removed core_review labels May 15, 2019

JonTheNiceGuy added some commits May 15, 2019

@ansibot ansibot added needs_ci stale_ci and removed needs_ci labels May 15, 2019

@ansibot ansibot removed the stale_ci label May 15, 2019

@JonTheNiceGuy

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

OK @acozine / @felixfontein I think I've sorted everything now?

Update lib/ansible/modules/net_tools/basics/uri.py
Co-Authored-By: Felix Fontein <felix@fontein.de>

Requested changes have been addressed - thanks @felixfontein for the review!

@acozine acozine merged commit 77f3e84 into ansible:devel May 20, 2019

1 check passed

Shippable Run 123077 status is SUCCESS.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.