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

logger added to adapter.py #93

Merged
merged 2 commits into from
May 21, 2019
Merged

Conversation

popokatapepel
Copy link
Contributor

enables the user to use l=logging.getLogger('requests_mock.adapters') to log all mocked requests and responses, similar to using l=logging.getLogger('urllib3.connectionpool')

@@ -245,6 +249,7 @@ def send(self, request, **kwargs):
if resp is not None:
request._matcher = weakref.ref(matcher)
resp.connection = self
logger.debug('{} \"{} \" {}'.format(request._request.url,request._request.method, resp.status_code))
Copy link
Owner

Choose a reason for hiding this comment

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

Where did the format come from? The space after method seems weird. It prints like:

DEBUG:requests_mock.adapter:http://test.com/ "GET " 200

I would think it would either follow apache format, or maybe GET http://test.com 200.

@jamielennox
Copy link
Owner

Hey thanks for the PR - I love the idea. Question on the format inline as it seems to be referring to something that I'm not aware of.

Am happy to merge some version of this, though I wasn't planning another release for a little while.

@popokatapepel
Copy link
Contributor Author

Hi,
the log format is trying to mimic the log that is generated by urllib3.connectionpool which looks like this

´INFO:requests.packages.urllib3.connectionpool:Starting new HTTP connection (1): www.google.de
DEBUG:requests.packages.urllib3.connectionpool:"GET / HTTP/1.1" 200 4848 ´

However, I could also change it to the apache format. If you are not planning on releasing for a while could also close the PR, I can create an enhancement request and we are looking for other places where looging might be useful. Afterwards I could implement it and create a new PR.

Would that be a good idea?

@jamielennox
Copy link
Owner

We can do an enhancement request, but it's probably a bit overkill for a project like this. I'm aware of all the PRs that are lodged (even if it's taking me a bit more time to respond these days). If we can agree on a format then just open PRs wherever you think they are appropriate and we can debate them there.

We don't really have all the information required to implement either apache or the urllib3 connection string format (We don't know http version or anything, and we should log the whole path). I just think the "GET " with the space looks funny.

I'd prefer the method url status format, but let's just get started :)

@popokatapepel
Copy link
Contributor Author

ok i changed it to the format.. integrating an empty http version doesn't really make sense

@jamielennox jamielennox merged commit cb894f1 into jamielennox:master May 21, 2019
openstack-gerrit pushed a commit to openstack/keystoneauth that referenced this pull request Sep 6, 2019
With the requests-mock logger now configured to log the request[1],
checking that the logger output does *not* contain the request is
invalid. Simplify these two tests by omitting the assertion.

[1] jamielennox/requests-mock#93

Closes-bug: #1842978

Change-Id: If3c0447502917bce831d3e9f7ae4c31374dd4380
openstack-gerrit pushed a commit to openstack/openstack that referenced this pull request Sep 6, 2019
* Update keystoneauth from branch 'master'
  - Simplify session logger object tests
    
    With the requests-mock logger now configured to log the request[1],
    checking that the logger output does *not* contain the request is
    invalid. Simplify these two tests by omitting the assertion.
    
    [1] jamielennox/requests-mock#93
    
    Closes-bug: #1842978
    
    Change-Id: If3c0447502917bce831d3e9f7ae4c31374dd4380
openstack-gerrit pushed a commit to openstack/python-keystoneclient that referenced this pull request Sep 9, 2019
Now requests-mock records request url in log[1], so it is
invalid to check that the logger output does NOT contain
request url.

Also, fix url passed to request mock as now it requires
complete url is passed

[1] jamielennox/requests-mock#93

Change-Id: I4bab30a6705b7cab6b5a569dd61c442263e39995
openstack-gerrit pushed a commit to openstack/openstack that referenced this pull request Sep 9, 2019
* Update python-keystoneclient from branch 'master'
  - Fix unit tests broken by requests-mock
    
    Now requests-mock records request url in log[1], so it is
    invalid to check that the logger output does NOT contain
    request url.
    
    Also, fix url passed to request mock as now it requires
    complete url is passed
    
    [1] jamielennox/requests-mock#93
    
    Change-Id: I4bab30a6705b7cab6b5a569dd61c442263e39995
openstack-gerrit pushed a commit to openstack/requirements that referenced this pull request Sep 19, 2019
Version 1.7.0 includes a fix that is breaking some keystoneauth tests:

  jamielennox/requests-mock#93

Cap at 1.6.0 until we can get keystoneauth refactored to use the new
version.

Related-Bug: #1842978
Change-Id: I45e2011424c45ce41387164d29f7efd90efdfe63
openstack-gerrit pushed a commit to openstack/openstack that referenced this pull request Sep 19, 2019
* Update requirements from branch 'master'
  - Merge "Cap requests-mock to 1.6.0"
  - Cap requests-mock to 1.6.0
    
    Version 1.7.0 includes a fix that is breaking some keystoneauth tests:
    
      jamielennox/requests-mock#93
    
    Cap at 1.6.0 until we can get keystoneauth refactored to use the new
    version.
    
    Related-Bug: #1842978
    Change-Id: I45e2011424c45ce41387164d29f7efd90efdfe63
tanaypf9 pushed a commit to tanaypf9/pf9-requirements that referenced this pull request May 20, 2024
Version 1.7.0 includes a fix that is breaking some keystoneauth tests:

  jamielennox/requests-mock#93

Cap at 1.6.0 until we can get keystoneauth refactored to use the new
version.

Change-Id: I45e2011424c45ce41387164d29f7efd90efdfe63
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants