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.

Sign up for GitHub

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

Jump to bottom

Fix getheaders regression with open_url in Scaleway dynamic inventory #48671

Merged
merged 2 commits into from
Nov 21, 2018
Merged

Fix getheaders regression with open_url in Scaleway dynamic inventory #48671

merged 2 commits into from
Nov 21, 2018

Conversation

remyleone
Copy link
Contributor

SUMMARY

It seems that the response object does not have a getheader but it got a headers attribute.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • Scaleway dynamic inventory

@remyleone remyleone changed the title Fix regression Fix getheaders regression with open_url in Scaleway dynamic inventory Nov 14, 2018
@ansibot
Copy link
Contributor

ansibot commented Nov 14, 2018

Hi @sieben, thank you for submitting this pull-request!

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 14, 2018

cc @Spredzy @abarbare @Anthony25 @hekonsek @pilou-
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. inventory Inventory category needs_triage Needs a first human triage before being processed. scaleway small_patch support:community This issue/PR relates to code supported by the Ansible community. labels Nov 14, 2018
sivel
sivel approved these changes Nov 14, 2018
View reviewed changes
lib/ansible/plugins/inventory/scaleway.py
@@ -113,7 +113,7 @@ def _fetch_information(token, url):
except KeyError:
raise AnsibleError("Incorrect format from the Scaleway API response")

link = response.getheader('Link')
link = response.headers['Link']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a difference between py2/py3. Your change will work fine in both, and it still supports case insensitive key lookups.

Just for educational purposes, the getheader method exists at the following locations in py2/py3:

py2

response.headers.getheader

py3

response.getheader

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Nov 14, 2018
@remyleone
Copy link
Contributor Author

@sivel Could you merge it?

@remyleone
Copy link
Contributor Author

LGTM

@hekonsek
Copy link
Contributor

hekonsek commented Nov 14, 2018 •
edited by mattclay

please remove me from CC in Ansible issues and stop sending me notification emails.

@remyleone
Copy link
Contributor Author

@hekonsek Remove yourself from this line https://github.com/ansible/ansible/blob/devel/.github/BOTMETA.yml#L1147

@ansibot ansibot added the bot_broken The bot is misbehaving. NOT for failing CI. A staff member will investigate. label Nov 14, 2018
@pilou- pilou- mentioned this pull request Nov 14, 2018
Merged
@abarbare
Copy link
Contributor

Shipit

@sivel
Copy link
Member

sivel commented Nov 15, 2018

@sieben I assume this should be backported, and to do so we will need a changelog fragment added in changelog/fragments. Could you add one? Once added I can merge.

@ansible ansible deleted a comment from hekonsek Nov 16, 2018
@mattclay mattclay removed the bot_broken The bot is misbehaving. NOT for failing CI. A staff member will investigate. label Nov 16, 2018
@remyleone
Copy link
Contributor Author

@sivel done. Could you merge?

@remyleone
Copy link
Contributor Author

@sivel Could this be merged?

@sivel
Copy link
Member

sivel commented Nov 21, 2018

@sieben apologies for not merging yet. The format of the changelog needed updated, which I did yesterday, but needed CI to run. I did not have time to come back and confirm yesterday.

@sivel sivel merged commit 959395f into ansible:devel Nov 21, 2018
mjmayer pushed a commit to mjmayer/ansible that referenced this pull request Nov 30, 2018
@remyleone @mjmayer
Fix getheaders regression with open_url in Scaleway dynamic inventory (…
8cedd5c 
…ansible#48671)

* Fix regression

* Improve changelog fragment
Tomorrow9 pushed a commit to Tomorrow9/ansible that referenced this pull request Dec 4, 2018
@remyleone @Tomorrow9
Fix getheaders regression with open_url in Scaleway dynamic inventory (…
66cb8e1 
…ansible#48671)

* Fix regression

* Improve changelog fragment
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sivel sivel sivel approved these changes

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. inventory Inventory category scaleway support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@remyleone @ansibot @hekonsek @abarbare @sivel @mattclay