bug: only require CI=true and AccessToken when making API requests#1278
bug: only require CI=true and AccessToken when making API requests#1278
Conversation
| return c.inCI | ||
| } | ||
|
|
||
| func (c *config) requireCIAccessToken() error { |
There was a problem hiding this comment.
this needs a doc string. Additionally at callsites this reads kinda weird. Maybe it should be called checkIfCIAccessTokenRequired (matching up with the err it returns)
|
|
||
| func (c *config) requireCIAccessToken() error { | ||
| if c.InCI() && c.AuthMode() != AuthModeAccessToken { | ||
| return errCIAccessTokenRequired |
There was a problem hiding this comment.
there might be an access token set but it is oauth instead? Maybe we need to include a little more context when this fails to help debug weird issues. Or does oauth + CI seem impossible?
There was a problem hiding this comment.
I'll add some context
Or does oauth + CI seem impossible?
Not entirely impossible but it's definitely the more difficult path. If there is no keyring the OAuth token you get will only be used for that session and then forgotten (we do warn when this happens)
There was a problem hiding this comment.
We also can't use OAuth access tokens as SRC_ACCESS_TOKEN values, since OAuth requires Bearer in the Authorization header and SRC_ACCESS_TOKEN requires token
There was a problem hiding this comment.
Updated to provide more context in the error!
- rename `RequireAccessToken` to `checkIfCIAccessTokenRequired` - provide more context in error on why it happened
e6c2532 to
cd32acb
Compare
In https://github.com/sourcegraph/sourcegraph/actions/runs/23518169374/job/68455311352?pr=11118 the action failed on command
src version -client-only, where it should not have failed because it's a client only check and no API interaction thus not requiring a Access Token to be set.In this PR we update the config:
Update APIClient:
Closes CPL-291
Test plan
unit tests + manual tests