1213857 - Make using HawkAuth with the client more intuitive

Status: RESOLVED FIXED

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

Description

[Ed Morley [:emorley]]

See bug 1209555 comment 5.

Using HawkAuth with the client requires a little more boilerplate (and in not as clean a form as I'd like), so we need to decide what to do before documenting it and then asking people to migrate.

The options:

1) Use HawkAuth as is.

2) Try to improve upstream (see https://github.com/mozilla-services/requests-hawk/issues/8)

3) Provide a wrapper around HawkAuth ourselves (see https://github.com/mozilla/treeherder/compare/hawkauth-wrapper)

Comment 1

[Ed Morley [:emorley]]

Attachment: Allow passing Hawk client_id/secret directly to the client

I think I prefer this approach over making users of the client deal with the auth directly. I know things like requests make people pass in an `auth` object, but I think that is something that should be abstracted away from the user for treeherder-client.

This approach means we maintain maximum control over the use of HawkAuth, and can keep a stable API for users of the client, if we choose to switch to another library or HawkAuth adds support for the simpler constructor mentioned in https://github.com/mozilla-services/requests-hawk/issues/8 (and we don't then have to block on that GitHub issue).

Comment 2

[William Lachance (:wlach)]

Comment on attachment 8673353 [details] [review]
Allow passing Hawk client_id/secret directly to the client

I like this approach.

Maybe some tests would be nice?

At some point in the future we should replace the deprecation warnings with exceptions.

Comment 3

[Ed Morley [:emorley]]

(In reply to William Lachance (:wlach) from comment #2)
> I like this approach.
> 
> Maybe some tests would be nice?

Agreed, whilst the etl tests do exercise this code, I've also added a test to test_treeherder_client.py and updated the PR.

> At some point in the future we should replace the deprecation warnings with exceptions.

Shortly they will be removed entirely - I was just wanting to release at least one version of the client that supports both forms of authentication, for easier transition. We haven't actually released a client to PyPI that supports Hawk auth yet - I was waiting until we had this bug fixed.

Comment 4

[Ed Morley [:emorley]]

(In reply to Ed Morley [:emorley] from comment #3)
> We haven't actually released a client to PyPI that
> supports Hawk auth yet - I was waiting until we had this bug fixed.

Well, in the form proposed here at least :-)

Comment 5

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/b212683e2fd9d19da82fa36aa4028c20ba3c8e92
Bug 1213857 - Fix treeherder-client's setup.py requirements

Since requests-hawk had leading whitespace, and missed the review
comment. We also need at least requests 2.4.3, since we use the `json`
argument to `requests.post()`.

https://github.com/mozilla/treeherder/commit/892a6d975182cc082eb317e3301dbf4ff355e3ea
Bug 1213857 - Allow passing Hawk client_id/secret directly to the client

So people don't have to deal with HawkAuth themselves. The existing
`auth` is deprecated and will be removed in the future.

https://github.com/mozilla/treeherder/commit/9f84698e470d3fe27e55201d7e68188344584aca
Bug 1213857 - Update tests still using TreeherderAuth to use client_id

TreeherderAuth is deprecated, so this switches these tests to use
the new hawk client_id and secret parameters instead.