-
Notifications
You must be signed in to change notification settings - Fork 46
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
upload_datafile: handling of the content (mime) type #142
base: main
Are you sure you want to change the base?
Conversation
…into opendp-user_endpoint
…e method use the default content type when encoding the POST request if it's not supplied explicitly. (gdcc#118)
Codecov Report
@@ Coverage Diff @@
## develop #142 +/- ##
========================================
Coverage 50.91% 50.91%
========================================
Files 5 5
Lines 1316 1316
========================================
Hits 670 670
Misses 646 646
Continue to review full report at Codecov.
|
Update: I left AUSSDA, so my funding for pyDataverse development has stopped. I want to get some basic funding to implement the most urgent updates (PRs, Bug fixes, maintenance work). If you can support this, please reach out to me. (www.stefankasberger.at). If you have feature requests, the same. Another option would be, that someone else helps with the development and / or maintenance. For this, also get in touch with me (or comment here). |
@landreev thanks for the reminder in #142 and this PR! I think the addition of this PR makes sense and enhances the utility of the library. We have been able to resolve a similar issue encountered when replacing files with #174, wherein we have switched to another library that does not send Can you sync this PR to the current |
@landreev hey, @JR-1991 and I talked about this PR a couple weeks ago (recording) and wanted to let you know that merge conflicts are due to pyDataverse switching from We suspect #118 may be fixed already (I just left a comment about this) but we think the ability to send an arbitrary MIME type might be a nice feature to have and we'd welcome a pull request if you have the time. |
Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of the pull request.
All Submissions
Describe your environment
Follow best practices
Describe the PR
There is currently no way to pass the content (mime) type to
upload_datafile()
(see #118). Also, when the multi-part POST form is created inside the method, NO content type is specified for the upload. This apparently fools Dataverse into defaulting to "text/plain", without attempting to use its normal type detection methods. In other words, in its current form, all files uploaded via pyDataverse end up with the content type "text/plain". Even when they are of types normally recognized by Dataverse (popular image types, etc). This defaulting behavior can and should be addressed on the Dataverse side. But it should be a good idea to fix it on the pyDataverse side as well. So this PR does 2 things:application/octet-stream
- a polite way to say "type unknown" - when creating a multi-part POST entry, like curl does; which then prompts Dataverse to at least attempt to identify the file more accurately. This is achieved by switching to the long notation of passing the file to therequests.post
method: from{"file": open(filename, "rb")}
to{"file": (filename, open(filename, "rb"), content_type)}
.On the Dataverse side this is tracked in IQSS/dataverse#8344
Closes #ISSUE_NUMBER
to the end of this pull requestTesting
Commits
Closes #ISSUE_NUMBER
in your commit messages to auto-close the issue that it fixes (if such).Others
Documentation contribution
Code contribution