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

Requests via httpx #174

Merged
merged 8 commits into from
Feb 22, 2024
Merged

Requests via httpx #174

merged 8 commits into from
Feb 22, 2024

Conversation

JR-1991
Copy link
Member

@JR-1991 JR-1991 commented Feb 8, 2024

Overview

As highlighted and discussed in issue #171, the requests library seems to supply wrong content types, leading to certain upload operations such as replace_datafile not working (see the issue for details). This PR addresses this issue and substitutes requests with httpx that fixes the highlighted problem.

Besides the previously mentioned fix, the library also provides means of async requests as shown here. Since async requests were planned anyway, this would inevitably also lead to ditching requests and thus the change to httpx already paves the way.

Changes introduced by this PR

  • Substituted base requests get_request, post_request, put_request and delete_request with httpx requests
  • Update requirements to include httpx>=0.26.0
  • Added tests for datafile upload and replacement

Additional notes

  • Due to httpx supporting Python >3.8, tests for 3.7 had to be removed

Closes

Fixes #171

@JR-1991 JR-1991 added pkg:testing test related activities pkg:api api related activities prio:asap Fix as soon as possible status:for review Waiting to be reviewd labels Feb 8, 2024
@JR-1991 JR-1991 self-assigned this Feb 8, 2024
@JR-1991 JR-1991 mentioned this pull request Feb 8, 2024
4 tasks
@JR-1991
Copy link
Member Author

JR-1991 commented Feb 14, 2024

Todo: Add a badge for Python versions supported

Add supported Python versions
@@ -1,4 +1,4 @@
[![PyPI](https://img.shields.io/pypi/v/pyDataverse.svg)](https://pypi.org/project/pyDataverse/) ![Build Status](https://github.com/gdcc/pyDataverse/actions/workflows/test_build.yml/badge.svg) [![Coverage Status](https://coveralls.io/repos/github/gdcc/pyDataverse/badge.svg)](https://coveralls.io/github/gdcc/pyDataverse) [![Documentation Status](https://readthedocs.org/projects/pydataverse/badge/?version=latest)](https://pydataverse.readthedocs.io/en/latest) ![PyPI - Python Version](https://img.shields.io/pypi/pyversions/pydataverse.svg) [![GitHub](https://img.shields.io/github/license/gdcc/pydataverse.svg)](https://opensource.org/licenses/MIT) [![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black) [![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.4664557.svg)](https://doi.org/10.5281/zenodo.4664557)
[![PyPI](https://img.shields.io/pypi/v/pyDataverse.svg)](https://pypi.org/project/pyDataverse/) ![Build Status](https://github.com/gdcc/pyDataverse/actions/workflows/test_build.yml/badge.svg) [![Coverage Status](https://coveralls.io/repos/github/gdcc/pyDataverse/badge.svg)](https://coveralls.io/github/gdcc/pyDataverse) [![Documentation Status](https://readthedocs.org/projects/pydataverse/badge/?version=latest)](https://pydataverse.readthedocs.io/en/latest) <img src="https://img.shields.io/badge/python-3.8 | 3.9 | 3.10 | 3.11-blue.svg" alt="PyPI - Python Version"> [![GitHub](https://img.shields.io/github/license/gdcc/pydataverse.svg)](https://opensource.org/licenses/MIT) [![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black) [![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.4664557.svg)](https://doi.org/10.5281/zenodo.4664557)
Copy link
Member

@pdurbin pdurbin Feb 14, 2024

Choose a reason for hiding this comment

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

As discussed, I copied this from https://github.com/gdcc/python-dvuploader (but changed the alt text).

I played a bit with a Markdown version...

![PyPI - Python Version](https://img.shields.io/badge/python-3.8|3.9|3.10|3.11-blue)

... but it's squished together and ugly to add %20 for spaces.

It's interesting that the old badge comes from https://img.shields.io/pypi/pyversions/pydataverse.svg . How are those versions set? 🤔 Should we be updating more files?

Copy link
Member Author

@JR-1991 JR-1991 Feb 16, 2024

Choose a reason for hiding this comment

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

Thanks for adding the badge @pdurbin !

True that, both ways do not look perfect. As far as I can recall, adding whitespaces is used by most Python repositories (e.g. pyDantic). Shall we stick to the whitespaces?

It's interesting that the old badge comes from https://img.shields.io/pypi/pyversions/pydataverse.svg . How are those versions set? 🤔 Should we be updating more files?

We dont need to specify the version using this badge. It fetches the latest version found on PyPI and puts it there. Pretty smooth 😁

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

As discussed during the https://py.gdcc.io meeting today, tests are passing and we want to add a badge to the README indicating that newer versions of Python are required. I went ahead and added a commit for this and some additional discussion but the PR looks good. Approved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:api api related activities pkg:testing test related activities prio:asap Fix as soon as possible status:for review Waiting to be reviewd
Projects
Development

Successfully merging this pull request may close these issues.

problem with replace_datafile
2 participants