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

refactor: Pass Accept header to requests in contrib.utils.download #1673

Merged
merged 6 commits into from
Nov 1, 2021

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Oct 26, 2021

Description

The HEPData landing page for the resource file can check if the Accept request HTTP header matches the content type of the resource file and return the content directly if so, for example, curl -LH "Accept: application/x-tar" https://doi.org/10.17182/hepdata.89408.v1/r2.

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Pass requests the `Accept` HTTP header that matches the content type
of the resource file. The HEPData landing page for the resource
requested can check if the header matches and then return the content
directly.
   - c.f. https://blog.datacite.org/changes-to-doi-content-negotiation/

Co-authored-by: Graeme Watt <Graeme.Watt@durham.ac.uk>

@matthewfeickert matthewfeickert added refactor A code change that neither fixes a bug nor adds a feature contrib Targeting pyhf.contrib and not the core of pyhf labels Oct 26, 2021
@matthewfeickert matthewfeickert self-assigned this Oct 26, 2021
@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #1673 (29050d3) into master (fbdce47) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1673   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files          64       64           
  Lines        4213     4213           
  Branches      585      585           
=======================================
  Hits         4131     4131           
  Misses         49       49           
  Partials       33       33           
Flag Coverage Δ
contrib 25.30% <0.00%> (ø)
doctest 61.04% <100.00%> (ø)
unittests 96.36% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pyhf/contrib/utils.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbdce47...29050d3. Read the comment docs.

@matthewfeickert
Copy link
Member Author

@GraemeWatt your review here is welcome as well (though I'm lifting your code from Issue #1491).

The pyhf team will address the rest of what's covered in Issue #1672 in follow up PRs.

@GraemeWatt
Copy link
Contributor

Thanks, looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contrib Targeting pyhf.contrib and not the core of pyhf refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass Accept header in contrib.utils.download
3 participants