Skip to content

add API key support in Hermes and Benchmarks clients#6

Open
bearmigiano wants to merge 10 commits into
mainfrom
feat/hermes-api-key
Open

add API key support in Hermes and Benchmarks clients#6
bearmigiano wants to merge 10 commits into
mainfrom
feat/hermes-api-key

Conversation

@bearmigiano

Copy link
Copy Markdown
Collaborator
  • Fix bug which returns a new http client instead of the configured one
  • Refactoring clients to use shared HTTP base client

@bearmigiano bearmigiano requested a review from ewokndor June 17, 2026 13:18

@ewokndor ewokndor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This lgtm. Just one note:
If we are going to deploy this into k8s, then we might be required to load passwords from file, not env vars. You might want to consider supporting this right away.

@ewokndor

Copy link
Copy Markdown
Contributor

However, note that the tests are not passing

@bearmigiano

Copy link
Copy Markdown
Collaborator Author

@ewokndor good point in general, but I think it doesn't apply here: this module isn't a deployable, atm it's just a dependency consumed by autobera, so nothing here ever runs in k8s on its own.
The API key only shows up in the tests (via env var for CI); the library itself just takes the key as a config string.


env:
GOPATH: /home/runner/go
PYTH_API_KEY: ${{ secrets.PYTH_API_KEY }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we actually run real tests against pyth? Because this would require to add the API_KEY just for tests? That feels awkward, if that is what's happening, we should mock the according test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unless there's a reason for it which I am not seeing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK the client just calls pyth and nothing else, test makes no sense without


env:
GOPATH: /home/runner/go
PYTH_API_KEY: ${{ secrets.PYTH_API_KEY }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK the client just calls pyth and nothing else, test makes no sense without

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.

2 participants