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 RetryStacIO #986

Merged
merged 3 commits into from
Feb 15, 2023
Merged

Add RetryStacIO #986

merged 3 commits into from
Feb 15, 2023

Conversation

gadomski
Copy link
Member

@gadomski gadomski commented Feb 14, 2023

Related Issue(s):

Description:

Though I initially thought I could do this without urllib3, as I was implementing I decided it was silly to just copy the functionality from urllib3, since it's so commonly on systems w/ PySTAC. So I added it as an extras_require. The urllib3 typing story is a bit of a mess, so I had to use a couple of # type: ignores. Testing retries is more trouble than its worth IMO, so I only included smoke tests for reading with the new class.

Background:

requests uses urllib3, so I consider urllib3 a pretty no-brainer library to optionally depend on -- it's everywhere.

Sidecar:

  • Includes two small doc fixes I found along the way
  • Casually adds pystac-vcr as a test dependency, with an eye towards Avoid network calls in tests suite #949
  • I rearranged requirements-test.txt because its arbitrary layout was getting to me ... now it's just a sorted list

PR Checklist:

  • Code is formatted (run pre-commit run --all-files)
  • Tests pass (run scripts/test)
  • Documentation has been updated to reflect changes, if applicable
  • This PR maintains or improves overall codebase code coverage.
  • Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

@gadomski gadomski added this to the 1.7 milestone Feb 14, 2023
@gadomski gadomski requested a review from pjhartzell February 14, 2023 21:23
@gadomski gadomski self-assigned this Feb 14, 2023
@gadomski gadomski force-pushed the issues/958-retry-stac-io branch from 3bf9811 to da0ba73 Compare February 14, 2023 21:26
Copy link
Collaborator

@pjhartzell pjhartzell left a comment

Choose a reason for hiding this comment

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

Looks good.

@gadomski gadomski force-pushed the issues/958-retry-stac-io branch from da0ba73 to d4615ef Compare February 15, 2023 17:53
@gadomski gadomski enabled auto-merge February 15, 2023 17:54
@gadomski gadomski added this pull request to the merge queue Feb 15, 2023
Merged via the queue into main with commit be0b6c0 Feb 15, 2023
@gadomski gadomski deleted the issues/958-retry-stac-io branch February 15, 2023 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New feature: StacIO that retries network requests
2 participants