1245158 - Client should allow requests to share network connections

Status: RESOLVED FIXED

(Tree Management Graveyard :: Treeherder: Client Libraries, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Right now if you make several calls to the Treeherder API, each time a new network connection gets created. This is not necessary, given that requests can share an already open connection. 

http://docs.python-requests.org/en/latest/user/advanced/#session-objects

Comment 1

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:pyclient-use-session > mozilla:master

Comment 2

[Ed Morley [:emorley]]

Comment on attachment 8762533 [details] [review]
[treeherder] mozilla:pyclient-use-session > mozilla:master

This should be a reasonable win even just for Treeherder itself (given how we batch eg job ingestion).

Comment 3

[William Lachance (:wlach)]

Comment on attachment 8762533 [details] [review]
[treeherder] mozilla:pyclient-use-session > mozilla:master

lgtm!

Comment 4

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/e22ec129a044e90ceb274ecd6b2535f22773f9bf
Bug 1245158 - Use responses instead of mock in the Python client tests

Since it's cleaner, works automatically with requests sessions and also
ensures that the requested URL was what was expected.

https://github.com/mozilla/treeherder/commit/3fc810934e3f37c80241e2f7cd8b40ca423e5925
Bug 1245158 - Make Python client requests use connection pooling

Keep-alive/connection pooling increases performance when making multiple
requests to the same host. The requests library automatically enables
urllib3's connection pooling when using a `Session()`:
http://docs.python-requests.org/en/latest/user/advanced/#session-objects

After this change, each `TreeherderClient()` instance will reuse a
requests session (the underlying urllib3 intelligently handles how long
to keep each connection open for / reopening if it dropped), which gives
a win, since often multiple requests are made by a single instance
of the client. For example:
https://github.com/mozilla/treeherder/blob/e824b69249d9e417b98310a1c49e4bb49b3bd7b1/treeherder/etl/th_publisher.py#L18-L41
https://github.com/mozilla/mozmill-ci/blob/7f1a04bbbf6612a6b21afc94fe7756b681f5e88c/jenkins-master/jobs/trigger-ondemand/workspace/trigger.py#L78
https://github.com/avih/talos_validator/blob/5e8125be0cfe314f932aac2e4c174e8e750d2a63/test-client.py#L30
https://github.com/mozilla/mozdownload/blob/0cbc6a183a613ee553a4a1759eae72155f1468b3/mozdownload/treeherder.py#L47
https://github.com/mozilla/mozilla_ci_tools/blob/735f5a4ffd38f6dccb40727d7985b9f8bd23e7f0/mozci/query_jobs.py#L226
https://github.com/mozilla/pulse_actions/blob/bba2ce71c191ea936b753c3c8b3821275724ae4d/pulse_actions/handlers/treeherder_buildbot.py#L45-L61

https://github.com/mozilla/treeherder/commit/8018695367c0943238d8a603a1f064449fd5e843
Bug 1245158 - Send auth headers for Python client GETs too

Previously, if the `client_id` and `secret` were set, the Hawk auth
headers would only be sent for requests using `_post_json()`. Now they
will also be sent when using `_get_json()` too.

This change means:
1) For users already setting `client_id` & `secret` for GET-based client
functionality, we can more easily tell from where requests are coming
(since transactions in New Relic are annotated with the `client_id`).
2) In the future bug 1203556 can implement rate limiting for GETs too,
with higher thresholds for authenticated users.