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

Add ability to retry to Client #124

Merged
merged 2 commits into from Apr 17, 2020
Merged

Add ability to retry to Client #124

merged 2 commits into from Apr 17, 2020

Conversation

nlipsyc
Copy link
Contributor

@nlipsyc nlipsyc commented Apr 15, 2020

Add an optional argument to the gapi client that will retry failed queries. This is useful as a workaround for the frequent timeouts TinCan is facing that result in an empty query being returned. The hope is that on the next try it will complete the query within the alloted time.

Under this hood, this creates a urllib3 Retry object in the requests' HTTPAdapter. If someone wanted more granular control, it's totally possible to pass other params like backoff_factor to that object.

Add an optional argument to the gapi client that will retry failed queries.  This is useful as a workaround for the frequent timeouts TinCan is facing that result in an empty query being returned. The hope is that on the next try it will complete the query within the alloted time.

Under this hood, this creates a urllib3 `Retry` object in the requests' `HTTPAdapter`.  If someone wanted more granular control, it's totally possible to pass other params like backoff_factor to that object.
Copy link
Contributor

@marz619 marz619 left a comment

Choose a reason for hiding this comment

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

@nlipsyc LGTM! Only addition is to add an env variable for max_retries

gapipy/client.py Outdated Show resolved Hide resolved
Copy link
Contributor

@marz619 marz619 left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@marz619 marz619 removed the request for review from silent1mezzo April 17, 2020 15:34
@marz619 marz619 merged commit d9bc24c into master Apr 17, 2020
@marz619 marz619 deleted the add-retry-options branch April 17, 2020 15:57
marz619 added a commit that referenced this pull request Apr 20, 2020
* Fix for `2.26.0 (2020-04-14)`_ and `Issue #113`_
  * Calls to ``Query.all`` will use initialised its parameters, unless the URI
    provides its own.
  * See `PR 123 <https://github.com/gadventures/gapipy/pull/123>`_
* Add the ability to define the ``max_retries`` values on the requestor.
  * New ``env`` value ``GAPI_CLIENT_MAX_RETRIES``
  * The default value will be ``0``, and if provided will override the ``retry``
    value on the ``requests.Session``.
  * This change will also always initialize a ``requests.Session`` value on
    initialisation of the ``gapipy.Client``.
  * See `PR 124 <https://github.com/gadventures/gapipy/pull/124>`_
* Add ``variation_id`` field to the ``Image`` resource.
  * See `Commit edc8d9b <https://github.com/gadventures/gapipy/commit/edc8d9b>`_
* Update the ``ActivityDossier`` and ``AccommodationDOssier`` resources.
  * Remove the ``is_prepaid`` field
  * Adds the ``has_costs`` field
  * See `Commit bd35531 <https://github.com/gadventures/gapipy/commit/bd35531>`_

.. _`Issue #113`: #113
marz619 added a commit that referenced this pull request Apr 20, 2020
* Fix for `2.26.0 (2020-04-14)`_ and `Issue #113`_.
  * Calls to ``Query.all`` will use initialised its parameters, unless the URI
  provides its own.
  * See `PR 123 <https://github.com/gadventures/gapipy/pull/123>`_.
* Add the ability to define the ``max_retries`` values on the requestor.
  * New ``env`` value ``GAPI_CLIENT_MAX_RETRIES``.
  * The default value will be ``0``, and if provided will override the ``retry``
  value on the ``requests.Session``.
  * This change will also always initialize a ``requests.Session`` value on
  initialisation of the ``gapipy.Client``.
  * See `PR 124 <https://github.com/gadventures/gapipy/pull/124>`_.
* Add ``variation_id`` field to the ``Image`` resource.
  * See `Commit edc8d9b <https://github.com/gadventures/gapipy/commit/edc8d9b>`_.
* Update the ``ActivityDossier`` and ``AccommodationDOssier`` resources.
  * Remove the ``is_prepaid`` field.
  * Adds the ``has_costs`` field.
  * See `Commit bd35531 <https://github.com/gadventures/gapipy/commit/bd35531>`_.

.. _`Issue #113`: #113
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.

None yet

3 participants