support env vars for options in server config #364
Conversation
server/config.go
Outdated
| } | ||
| if v, ok := os.LookupEnv("POLICYBOT_STATUS_CHECK_CONTEXT"); ok { | ||
| c.Options.StatusCheckContext = v | ||
| } |
There was a problem hiding this comment.
This would work right? @bluekeyes
| setStringFromEnv("SHARED_POLICY_PATH", prefix, &p.SharedPolicyPath) | ||
| setStringFromEnv("STATUS_CHECK_CONTEXT", prefix, &p.StatusCheckContext) | ||
| p.fillDefaults() | ||
| } |
There was a problem hiding this comment.
i think maybe this is the better way mirroring your pattern from go-baseapp and go-githubapp, but i felt like these options weren't suited to live in those codebases. so i put them here instead.
only thing i felt weird about was setting the defaults here, probably fine, but let me know what you think
There was a problem hiding this comment.
I agree it's a bit weird, but I think it's fine since this is only used in one place.
| @@ -126,7 +126,8 @@ define a `remote` key. Only 1 level of remote configuration is supported by desi | |||
|
|
|||
| ```yaml | |||
| # The remote repository to read the policy file from. This is required, and must | |||
| # be in the form of "org/repo-name". Must be a public repository. | |||
| # be in the form of "org/repo-name". The policy bot github app must have read | |||
| # access to this repository. | |||
There was a problem hiding this comment.
i added this because it was little misleading
There was a problem hiding this comment.
Thanks, that's probably a bit clearer... hopefully it is implied that apps have read access to public repositories.
devinburnette
force-pushed
the
set-options-from-env-vars
branch
from
5aa1ddd to
73e29f0
Compare
| `policy-bot` works with private repositories, but currently does not support | ||
| pull requests from private _forks_ of private repositories due to GitHub API | ||
| limitations. Please file an issue if this functionality is important to you. | ||
|
|
There was a problem hiding this comment.
removing this, because I don't think it's true anymore. private forks of private repos seem to work fine on my end.
There was a problem hiding this comment.
Interesting, this was originally added due to this fallback condition when loading commit pushed dates. Maybe GitHub improved the GraphQL API so that pushed dates are returned from forks now and we don't need the fallback.
| @@ -126,7 +126,8 @@ define a `remote` key. Only 1 level of remote configuration is supported by desi | |||
|
|
|||
| ```yaml | |||
| # The remote repository to read the policy file from. This is required, and must | |||
| # be in the form of "org/repo-name". Must be a public repository. | |||
| # be in the form of "org/repo-name". The policy bot github app must have read | |||
| # access to this repository. | |||
There was a problem hiding this comment.
Thanks, that's probably a bit clearer... hopefully it is implied that apps have read access to public repositories.
| `policy-bot` works with private repositories, but currently does not support | ||
| pull requests from private _forks_ of private repositories due to GitHub API | ||
| limitations. Please file an issue if this functionality is important to you. | ||
|
|
There was a problem hiding this comment.
Interesting, this was originally added due to this fallback condition when loading commit pushed dates. Maybe GitHub improved the GraphQL API so that pushed dates are returned from forks now and we don't need the fallback.
| setStringFromEnv("SHARED_POLICY_PATH", prefix, &p.SharedPolicyPath) | ||
| setStringFromEnv("STATUS_CHECK_CONTEXT", prefix, &p.StatusCheckContext) | ||
| p.fillDefaults() | ||
| } |
There was a problem hiding this comment.
I agree it's a bit weird, but I think it's fine since this is only used in one place.
Closes #363
As discussed in the attached issue, I've made an attempt at setting the policy path and shared options from env vars if they exist. If env vars are not set, then it should just fall back to the defaults. Let me know if there's a more clever way of accomplishing the same. I also updated the example config to reflect the possibilities.