All users were logged out of Bugzilla on October 13th, 2018
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)
Created attachment 8673353 [details] [review] 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).
Attachment #8673353 - Flags: review?(mdoglio)
Attachment #8673353 - Flags: feedback?(wlachance)
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.
Attachment #8673353 - Flags: feedback?(wlachance) → feedback+
Attachment #8673353 - Flags: review?(mdoglio) → review+
(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.
(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 :-)
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.