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

Change code host connection checks to talk HTTP#46918

Merged
mrnugget merged 3 commits into
mainfrom
mrn/http-connection-check
Jan 25, 2023
Merged

Change code host connection checks to talk HTTP#46918
mrnugget merged 3 commits into
mainfrom
mrn/http-connection-check

Conversation

@mrnugget

Copy link
Copy Markdown
Contributor

Full context: https://github.com/sourcegraph/sourcegraph/issues/46915

This changes the connection checks to test what we're after: can we talk to the code hosts' API via HTTP?

Because as it turns out, in some customer environments we can't do a TCP dial, because proxies block it. In that case an HTTP call makes even more sense, because if that succeeds we know we can talk to the API.

The other change in here is to not hard-fail the repository syncing if the connection check fails. Until we're confident that the checks work exactly as we want them to, we only print a warning.

cc @varsanojidan we need to do something similar for ADO but we didn't do that in this PR because we don't have an account yet and aren't confident in our abilities to properly test this (which we should for a patch release). And since ADO is not out in the wild yet.

Test plan

  • Manual testing for Gitlab.com, gitlab.sgdev.org, GitHub.com, github.sgdev.org, bitbucket.org, bitbucket.sgdev.org
  • Existing unit tests for the client functions

Full context: https://github.com/sourcegraph/sourcegraph/issues/46915

This changes the connection checks to test what we're after: can we talk
to the code hosts' API via HTTP?

Because as it turns out, in some customer environments we can't do a TCP
dial, because proxies block it. In that case an HTTP call makes even
more sense, because if that succeeds we know we can talk to the API.

The other change in here is to not hard-fail the repository syncing if
the connection check fails. Until we're confident that the checks work
exactly as we want them to, we only print a warning.
@mrnugget mrnugget requested review from a team and kopancek January 25, 2023 10:20
@cla-bot cla-bot Bot added the cla-signed label Jan 25, 2023
@sourcegraph-bot

sourcegraph-bot commented Jan 25, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 8a9806a...23e2df9.

Notify File(s)
@indradhanush internal/repos/bitbucketcloud.go
internal/repos/bitbucketserver.go
internal/repos/github.go
internal/repos/gitlab.go
internal/repos/syncer.go
internal/repos/syncer_test.go
@ryanslade internal/repos/bitbucketcloud.go
internal/repos/bitbucketserver.go
internal/repos/github.go
internal/repos/gitlab.go
internal/repos/syncer.go
internal/repos/syncer_test.go
@sashaostrikov internal/repos/bitbucketcloud.go
internal/repos/bitbucketserver.go
internal/repos/github.go
internal/repos/gitlab.go
internal/repos/syncer.go
internal/repos/syncer_test.go

Comment thread CHANGELOG.md Outdated
@mrnugget mrnugget merged commit b7f51e3 into main Jan 25, 2023
@mrnugget mrnugget deleted the mrn/http-connection-check branch January 25, 2023 10:52
mrnugget added a commit that referenced this pull request Jan 25, 2023
Full context: https://github.com/sourcegraph/sourcegraph/issues/46915

This changes the connection checks to test what we're after: can we talk
to the code hosts' API via HTTP?

Because as it turns out, in some customer environments we can't do a TCP
dial, because proxies block it. In that case an HTTP call makes even
more sense, because if that succeeds we know we can talk to the API.

The other change in here is to not hard-fail the repository syncing if
the connection check fails. Until we're confident that the checks work
exactly as we want them to, we only print a warning.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants