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

feat: custom headers #76

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

comatory
Copy link

@comatory comatory commented Jan 28, 2025

Addresses #45

The configuration can contain a headers object, which will be applied to outgoing requests. I'm not a Python developer, so it is possible the code has issues. I just created a base test case.

I'm not sure if headers should be validated in any way, I figured it's ok for people to possibly override other built-int headers if they wish so.

My primary use case is to have Authorization header.

Change Summary

Added headers to config loader.

PR Checklist

The configuratiopn can contain `headers` object which will be applied
to outgoing request.
@jasonbosco
Copy link
Member

jasonbosco commented Jan 28, 2025

Thank you for the PR!

Looks like tests are failing, could you take a look?

@comatory
Copy link
Author

Thank you for the PR!

Looks like tests are failing, could you take a look?

I'm not completely certain it's issues with tests, it hasn't even got to tests. I cannot re-run the pipelines (it must be approved first), it would be useful to re-run them with debugging because the provided error does not help very much:

Error: Process completed with exit code 1.

It actually fails in "Install dependencies" step. To me it seems like CI might be misconfigured? I'm not sure if the last lines of the last log correspond to actual error:

Warning: Python 3.10 was not found on your system...
Neither 'pyenv' nor 'asdf' could be found to install Python.
You can specify specific versions of Python with:
$ pipenv --python path/to/python

Can you try opening PR and see if CI works first?

@tharropoulos
Copy link
Contributor

After merging #77, you should be able to rebase the ci changes into your branch and try again. It seems to be a problem with pipenv looking for a 3.10 installation and not being able to find one, since the 3.x version would actually be 3.12

@comatory
Copy link
Author

@tharropoulos ok I synced with upstream, can you please try and re-run the pipelines? 🙏

@comatory
Copy link
Author

I mentioned before that I'm not so familiar with Python and this repo overall. While the change seems easy enough, I haven't had a chance to test it. Would you be willing to try it out and see if custom headers are passed to the request(s)?

If everything works out well, would it be possible to release this change in a new image? I'd assume at least as a release candidate?

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.

3 participants