Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix installSrc in executors#57572

Merged
DaedalusG merged 2 commits into
mainfrom
wg/fix-executor-installSrc
Oct 16, 2023
Merged

fix installSrc in executors#57572
DaedalusG merged 2 commits into
mainfrom
wg/fix-executor-installSrc

Conversation

@DaedalusG

@DaedalusG DaedalusG commented Oct 12, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Use client for auth in HTTP request not options apiclient.EndpointOptions
Correctly adds auth header to request
internal context: https://sourcegraph.slack.com/archives/C01JR51JR5J/p1697050361943039

Test plan

Tested the request construction locally against sgdev:

λ ~/sourcegraph/ wg/fix-executor-installSrc EXECUTOR_FRONTEND_URL=https://k8s.sgdev.org EXECUTOR_FRONTEND_PASSWORD=REDACTED go run ./cmd/executor install src-cli --bin-dir ~
λ ~/sourcegraph/ wg/fix-executor-installSrc ~/src version
Current version: 5.2.0
Recommended version: 5.2.0 or later

Printing the new request

λ ~/sourcegraph/ wg/fix-executor-installSrc EXECUTOR_FRONTEND_URL=https://k8s.sgdev.org/ EXECUTOR_FRONTEND_PASSWORD=REDACTED go run ./cmd/executor install src-cli --bin-dir ~
Request:  &{GET https://k8s.sgdev.org/.api/src-cli/version HTTP/1.1 1 1 map[Authorization:[token-executor REDACTED] Content-Type:[application/json]] {0xc0002cf470} 0x1321dc0 4 [] false k8s.sgdev.org map[] map[] <nil> map[]   <nil> <nil> <nil> 0xc000124010}

Printing the old request

λ ~/sourcegraph/ main EXECUTOR_FRONTEND_URL=https://k8s.sgdev.org/ EXECUTOR_FRONTEND_PASSWORD=REDACTED go run ./cmd/executor install src-cli --bin-dir ~
Request:  &{GET https://k8s.sgdev.org/.api/src-cli/version HTTP/1.1 1 1 map[Content-Type:[application/json]] {0xc0002a2f30} 0x1321dc0 4 [] false k8s.sgdev.org map[] map[] <nil> map[]   <nil> <nil> <nil> 0xc00011c010}

@cla-bot cla-bot Bot added the cla-signed label Oct 12, 2023
@DaedalusG DaedalusG changed the title WIP: fix installSrc in executors fix installSrc in executors Oct 16, 2023
@DaedalusG DaedalusG marked this pull request as ready for review October 16, 2023 20:04
@DaedalusG DaedalusG merged commit 4012e57 into main Oct 16, 2023
@DaedalusG DaedalusG deleted the wg/fix-executor-installSrc branch October 16, 2023 21:29
@sourcegraph-release-bot sourcegraph-release-bot mentioned this pull request Jan 8, 2024
Merged
sourcegraph-release-bot pushed a commit that referenced this pull request Jan 8, 2024
@DaedalusG @github-actions
fix installSrc in executors (#57572)
e1af33e 
* untested refactor remove options apiclient.EndpointOptions

* possible fix after testing

(cherry picked from commit 4012e57)
DaedalusG added a commit that referenced this pull request Jan 9, 2024
@sourcegraph-release-bot @DaedalusG
[Backport 5.2] fix installSrc in executors (#59391)
ac23781 
fix `installSrc` in executors (#57572)

* untested refactor remove options apiclient.EndpointOptions

* possible fix after testing

(cherry picked from commit 4012e57)

Co-authored-by: Warren Gifford <warren@sourcegraph.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jdpleiness jdpleiness Awaiting requested review from jdpleiness

1 more reviewer

@eseliger eseliger eseliger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backport 5.2 cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DaedalusG @eseliger @jdpleiness