Allow httphandler to have auth and other mediatype#811
Allow httphandler to have auth and other mediatype#811NickSchouten wants to merge 7 commits intonteract:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #811 +/- ##
==========================================
- Coverage 91.54% 91.35% -0.20%
==========================================
Files 17 17
Lines 1621 1643 +22
==========================================
+ Hits 1484 1501 +17
- Misses 137 142 +5
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for HTTP authentication and custom Accept headers in the HTTPHandler through environment variables (PAPERMILL_HTTP_AUTH_HEADER and PAPERMILL_HTTP_ACCEPT_HEADER), enabling papermill to read from private repositories like Azure DevOps.
Key Changes:
- Added
_get_auth_kwargs()and_get_read_kwargs()helper methods to construct request headers from environment variables - Modified
read()andwrite()methods to use the new auth and accept header configurations - Added comprehensive test coverage for the new authentication and accept header functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| papermill/iorw.py | Implements helper methods to retrieve auth and accept headers from environment variables and integrates them into HTTP read/write operations |
| papermill/tests/test_iorw.py | Adds three new test cases covering auth-only, accept-header-only, and write-with-auth scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Gets the Authorization header from PAPERMILL_HTTP_AUTH_HEADER. | ||
| A valid example value Basic dW5hbWU6cGFzc3dvcmQK""" |
There was a problem hiding this comment.
The docstring is missing proper formatting. It should specify that this is a colon (:) after "value" or include it as part of the example. The current text reads "A valid example value Basic..." but should be "A valid example value: Basic..." or structured using proper docstring format with a Parameters or Example section.
| """Gets the Authorization header from PAPERMILL_HTTP_AUTH_HEADER. | |
| A valid example value Basic dW5hbWU6cGFzc3dvcmQK""" | |
| """ | |
| Gets the Authorization header from PAPERMILL_HTTP_AUTH_HEADER. | |
| Example | |
| ------- | |
| A valid example value: Basic dW5hbWU6cGFzc3dvcmQK | |
| """ |
|
|
||
| def test_read_with_auth(self): | ||
| """ | ||
| Tests that the `read` function performs a request to the giving path |
There was a problem hiding this comment.
Typo in the docstring: "giving" should be "given". This error appears in the original test at line 312 as well, but since this is new code being added, it should use the correct spelling.
|
|
||
| def test_read_with_accept_header(self): | ||
| """ | ||
| Tests that the `read` function performs a request to the giving path |
There was a problem hiding this comment.
Typo in the docstring: "giving" should be "given".
HTTPHandler can now be given auth headers and a different accept media type than application/json
I wanted to use papermill to directly read my git repo (azure devops). Since the repo is not publicly available I needed a way to pass auth to papermill while not breaking any current implementations.
In line with other setups in papermill I added an env variable PAPERMILL_HTTP_AUTH_HEADER that will be passed to requests like this {"headers" {"Authorization": PAPERMILL_HTTP_AUTH_HEADER}}. As such it should work for most auth setups. I only tested it for my use case (base64 encoded basic token).
I then also needed the reply to be "plain/text" instead of "application/json" so also added that option via an env var. PAPERMILL_HTTP_ACCEPT_HEADER
All relevant code is in iorw.py.
Wrote tests for all