Skip to content

Conversation

@NickSchouten
Copy link
Contributor

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

@NickSchouten NickSchouten changed the title WIP: Allow httphandler to have auth and other mediatype Allow httphandler to have auth and other mediatype Nov 20, 2024
@codecov
Copy link

codecov bot commented Nov 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.35%. Comparing base (cb2eb37) to head (630b373).
Report is 13 commits behind head on main.

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     
---- 🚨 Try these New Features:

@Borda Borda requested a review from Copilot November 13, 2025 21:02
Copilot finished reviewing on behalf of Borda November 13, 2025 21:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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() and write() 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.

Comment on lines +170 to +171
"""Gets the Authorization header from PAPERMILL_HTTP_AUTH_HEADER.
A valid example value Basic dW5hbWU6cGFzc3dvcmQK"""
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
"""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
"""

Copilot uses AI. Check for mistakes.

def test_read_with_auth(self):
"""
Tests that the `read` function performs a request to the giving path
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

def test_read_with_accept_header(self):
"""
Tests that the `read` function performs a request to the giving path
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Typo in the docstring: "giving" should be "given".

Copilot uses AI. Check for mistakes.
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.

1 participant