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

uri module enforces Headers in camel-case (explicit the documentation) #73702

Closed
wants to merge 1 commit into from
Closed

Conversation

axel3rd
Copy link

@axel3rd axel3rd commented Feb 23, 2021

SUMMARY

Origin: adapter-aws-lambda-serverless #62.

uri module enforce headers written in camel-case, even if filled otherwise (and written as filled in the debug logs).

The documentation should explicit that, avoiding tricky troubleshooting.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

uri

ADDITIONAL INFORMATION

Proof:

  1. Create a Python server which dump request which listen on 8000 port
  2. Write simple playbook and execute it
- name: Test
  hosts: localhost
  tasks:
  - name: Headers Proof
    uri:
      url: http://localhost:8000
      headers:
        X-SoMe-HeADer: FaKe-VaLue

Output produced:

Host: localhost:8000
X-Some-Header: FaKe-VaLue
Connection: close
User-Agent: ansible-httpget

@ansibot ansibot added affects_2.11 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 23, 2021
@sivel
Copy link
Member

sivel commented Feb 23, 2021

This is an implementation detail of how python transmits headers. We have no control over this behavior, and since it is an implementation detail which could change, and the RFC states that header names are case insensitive, I don't think this isn't something we should document.

@axel3rd
Copy link
Author

axel3rd commented Feb 23, 2021

@sivel : I understand the point of view.

But for avoid some lost of time in this kind of troubleshooting (They are written not modified in -vvv logs ^^), having a word about that somewhere could help IMO.

Perhaps adding a note could be relevant ?

notes:
- The dependency on httplib2 was removed in Ansible 2.1.
- The module returns all the HTTP headers in lower-case.
- For Windows targets, use the M(ansible.windows.win_uri) module instead.

 notes: 
   - The dependency on httplib2 was removed in Ansible 2.1. 
   - The module sends all the HTTP headers in camel-case. 
   - The module returns all the HTTP headers in lower-case. 
   - For Windows targets, use the M(ansible.windows.win_uri) module instead. 

?

@sivel
Copy link
Member

sivel commented Feb 23, 2021

The problem is that the behavior is dictated by Python, and it's not considered a feature, it's an implementation detail. The behavior is not mentioned at all in the Python docs. Get python to commit to it being a feature, and documenting the behavior, and I'd be open to accepting docs that point to that and convey it here.

Basically what I am saying, is that until python commits to that behavior being stable and documents it, we shouldn't either, because it should not be relied on as stable, and could change.

@axel3rd axel3rd marked this pull request as draft February 24, 2021 22:19
@axel3rd
Copy link
Author

axel3rd commented Feb 24, 2021

I will try to manage this strategy/documentation point in Python ecosystem.

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Feb 24, 2021
@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Feb 25, 2021
@jborean93
Copy link
Contributor

Closing as per the above, please reopen the PR if you get any response back from the CPython team.

@jborean93 jborean93 closed this Feb 25, 2021
@axel3rd
Copy link
Author

axel3rd commented Feb 27, 2021

For history: Python bug issue43331 / PR 24661.

@ansible ansible locked and limited conversation to collaborators Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.11 docs This issue/PR relates to or includes documentation. has_issue module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants