Skip to content
This repository has been archived by the owner on Apr 23, 2021. It is now read-only.

Use oauth session for user calls; support for throttling calls #35

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

superdosh
Copy link

What is in this PR?

Select all that apply

  • Bugfix

  • New feature

  • Documentation improvements

  • Improved test coverage

  • Other: (Please specify)

Issue

This might address the issue here, though that was not my goal: #34

Abstract

  1. Use the oauth session for user calls when available.
  2. Add support for throttling of API requests to stay within the API TOS (1 second between requests).

Dependency updates

None

Good Citizenship

  • I have updated any relevant tests and the test suite is passing

I could use some help thinking through the right way to test the oauth session user calls. I don't believe there is currently an authentication test. I tested for my own user after authorizing my app, so it works in that one case!

  • I have updated any relevant documentation

  • I have run the pre-commit hooks for this repository's designated code styles

* Currently, user calls like `per_shelf_reviews` fail for users without
public profiles. This is because the client is not using the authenticated
session.
@superdosh superdosh changed the title Request auth fix Use oauth session for user calls (when available); support for throttling calls Jan 3, 2020
@superdosh superdosh changed the title Use oauth session for user calls (when available); support for throttling calls Use oauth session for user calls; support for throttling calls Jan 3, 2020
* The Goodreads API TOS says to limit to no more than 1 call per second.
* This simple decorator allows the requests (in single-threaded applications)
to be throttled in accordance via setting an environmental variable.
* As a result of the changes that attempt to use the client session,
the GoodreadsComment test that tries to create the user started failing
since the GoodreadsComment class didn't preserve the client.
* This simple fix just preserves the client to preserve existing functionality.
@superdosh superdosh marked this pull request as ready for review January 3, 2020 05:09
@superdosh
Copy link
Author

@thejessleigh I'm not really sure how to replicate the build fail, so not sure what I need to fix. Any ideas?

@thejessleigh
Copy link
Owner

Hi! I'm so sorry I didn't see this. For some reason I haven't been getting email notifications about this repository. I'm looking at this tonight and hope to have a new version released over the weekend. Thank you for your patience!

@thejessleigh
Copy link
Owner

Hi! I'm so sorry for the delay. I took some time off of open source stuff due to work and personal reasons. I hope to start working through backlog PRs this month.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants