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

Introduce backups service. #87

Merged
merged 1 commit into from
May 25, 2024

Conversation

slobberchops
Copy link
Contributor

  • Support create(), get_full_list(), download() and delete()

  • Refactored Client.send() to support binary responses.

  • Added Client.send_raw() which returns raw bytes from HTTP response.

It was not possible to support any of the standard API CRUD operations provided by the BaseCrudService on account of the Backups API returning very different structures in most requests and responses. It would have required changing the interface for the class which seemed dangerous. Therefore, the API for Backups is a little more bespoke.

The tests for the Backups API is also a little different in that it seems like a somewhat heavier type of operation than other operations and lent itself better to doing verification that the underlying APIs were in fact called.

The download interface also has an optional parameter for file_token. Not providing one is easier to use, but could create a poor usage pattern as it is more inefficient for multiple download calls. If the reviewer could provide feedback as to whether it is desirable or acceptable it would be welcome.

* Support create(), get_full_list(), download() and delete()

* Refactored Client.send() to support binary responses.

* Added Client.send_raw() which returns raw bytes from HTTP response.
@slobberchops slobberchops mentioned this pull request May 11, 2024
@slobberchops
Copy link
Contributor Author

I have verified that test workflow runs and all tests pass for all supported Python versions.

@vaphes vaphes merged commit 42db364 into vaphes:master May 25, 2024
5 checks passed
@vaphes
Copy link
Owner

vaphes commented May 25, 2024

@slobberchops Thanks a lot for your contribution. Sorry for the wait.

@coveralls
Copy link

coveralls commented May 25, 2024

Pull Request Test Coverage Report for Build 9044392827

Details

  • 43 of 43 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 91.117%

Totals Coverage Status
Change from base Build 8850640839: 0.5%
Covered Lines: 834
Relevant Lines: 888

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 9044392827

Details

  • 43 of 43 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 91.117%

Totals Coverage Status
Change from base Build 8850640839: 0.5%
Covered Lines: 834
Relevant Lines: 888

💛 - Coveralls

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.

3 participants