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

factor out healthchecks API endpoint to an environment variable #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

earthquakesan
Copy link

I factored out the API endpoint URL into an env variable, this way CLI can be used with custom healthchecks deployment.

@cuu508
Copy link
Member

cuu508 commented Dec 11, 2019

Thanks for the PR!

CLI already needs to store API key:

hchk setkey my-api-key
# key gets saved in ~/.hchk

How about storing the API endpoint there as well?

Also I think we should only store the https://server/api/v1/ portion as the endpoint (without the checks/ bit at the end). This way it would be easier to add support for handling other resource types (e.g. integrations) in the future.

hchk setendpoint https://my-healthchecks-instance.net/api/v1/
# endpoint address gets saved in ~/.hchk

–something like that. What do you think?

@earthquakesan
Copy link
Author

Hi @cuu508,

I agree with the API endpoint notion, storing only https://server/api/v1/ is indeed better option.
However, saving API endpoint into the .hchk file exclusively is not exactly what I want. Environment variables usually is a more flexible option for deploying the CLI. You can find my usage scenario here (ansible):

https://github.com/earthquakesan/borg-lab/blob/master/playbooks/roles/hchk/tasks/main.yml

To have environment variables as a first class citizen in the code, we can define precedence as follows:

  • .hchk configuration file
  • env variables

And then both configuration options (API endpoint and key) can be defined either with env vars or with the configuration file. If env variable is set, configuration file is simply ignored.

Let me know what you think.

@cuu508
Copy link
Member

cuu508 commented Dec 11, 2019

Supporting configuration file and environment variables sounds good to me. Environment variables taking precedence also seems like the way to go. A couple quick spot checks:

Ansible prefers env vars over configuration file: https://docs.ansible.com/ansible/latest/reference_appendices/general_precedence.html
Boto3 prefers env vars over configuration file: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html

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