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.

Sign up for GitHub

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

Jump to bottom

Fix remote repository connection checking #2102

Merged
merged 6 commits into from Feb 1, 2019
Merged

Fix remote repository connection checking #2102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50d31ce
Fix remote repository connection checking
chrismwendt Feb 1, 2019
5f23f22
Avoid panic
chrismwendt Feb 1, 2019
e315dcc
Fix test
chrismwendt Feb 1, 2019
25cdd81
Switch to non-pointer string
chrismwendt Feb 1, 2019
5a046fc
s/Add/AddGitHubDotComRepository/
chrismwendt Feb 1, 2019
4f480e2
Use constants
chrismwendt Feb 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions cmd/frontend/backend/repos.go
View file
Open in desktop
Expand Up @@ -65,7 +65,7 @@ func (s *repos) GetByName(ctx context.Context, name api.RepoName) (_ *types.Repo
repo, err := db.Repos.GetByName(ctx, name)
if err != nil && envvar.SourcegraphDotComMode() {
chrismwendt marked this conversation as resolved.
Show resolved Hide resolved
// Automatically add repositories on Sourcegraph.com.
if err := s.Add(ctx, name); err != nil {
if err := s.AddGitHubDotComRepository(ctx, name); err != nil {
return nil, err
}
return db.Repos.GetByName(ctx, name)
Expand All @@ -84,21 +84,21 @@ func (s *repos) GetByName(ctx context.Context, name api.RepoName) (_ *types.Repo
return repo, nil
}

// Add adds the repository with the given name. The name is mapped to a repository by consulting the
// AddGitHubDotComRepository adds the repository with the given name. The name is mapped to a repository by consulting the
// repo-updater, which contains information about all configured code hosts and the names that they
// handle.
func (s *repos) Add(ctx context.Context, name api.RepoName) (err error) {
if Mocks.Repos.Add != nil {
return Mocks.Repos.Add(name)
func (s *repos) AddGitHubDotComRepository(ctx context.Context, name api.RepoName) (err error) {
if Mocks.Repos.AddGitHubDotComRepository != nil {
return Mocks.Repos.AddGitHubDotComRepository(name)
}

ctx, done := trace(ctx, "Repos", "Add", name, &err)
ctx, done := trace(ctx, "Repos", "AddGitHubDotComRepository", name, &err)
defer done()

// Avoid hitting the repoupdater (and incurring a hit against our GitHub/etc. API rate
// limit) for repositories that don't exist or private repositories that people attempt to
// access.
gitserverRepo, err := quickGitserverRepo(ctx, name)
gitserverRepo, err := quickGitserverRepo(ctx, name, "github.com")
if err != nil {
return err
}
Expand Down
16 changes: 8 additions & 8 deletions cmd/frontend/backend/repos_mock.go
View file
Open in desktop
Expand Up @@ -15,14 +15,14 @@ import (
)

type MockRepos struct {
Get func(v0 context.Context, id api.RepoID) (*types.Repo, error)
GetByName func(v0 context.Context, name api.RepoName) (*types.Repo, error)
Add func(name api.RepoName) error
List func(v0 context.Context, v1 db.ReposListOptions) ([]*types.Repo, error)
GetCommit func(v0 context.Context, repo *types.Repo, commitID api.CommitID) (*git.Commit, error)
ResolveRev func(v0 context.Context, repo *types.Repo, rev string) (api.CommitID, error)
GetInventory func(v0 context.Context, repo *types.Repo, commitID api.CommitID) (*inventory.Inventory, error)
GetInventoryUncached func(ctx context.Context, repo *types.Repo, commitID api.CommitID) (*inventory.Inventory, error)
Get func(v0 context.Context, id api.RepoID) (*types.Repo, error)
GetByName func(v0 context.Context, name api.RepoName) (*types.Repo, error)
AddGitHubDotComRepository func(name api.RepoName) error
List func(v0 context.Context, v1 db.ReposListOptions) ([]*types.Repo, error)
GetCommit func(v0 context.Context, repo *types.Repo, commitID api.CommitID) (*git.Commit, error)
ResolveRev func(v0 context.Context, repo *types.Repo, rev string) (api.CommitID, error)
GetInventory func(v0 context.Context, repo *types.Repo, commitID api.CommitID) (*inventory.Inventory, error)
GetInventoryUncached func(ctx context.Context, repo *types.Repo, commitID api.CommitID) (*inventory.Inventory, error)
}

var errRepoNotFound = &errcode.Mock{
Expand Down
2 changes: 1 addition & 1 deletion cmd/frontend/backend/repos_test.go
View file
Open in desktop
Expand Up @@ -86,7 +86,7 @@ func TestRepos_Add(t *testing.T) {
return nil
}

if err := s.Add(ctx, repoName); err != nil {
if err := s.AddGitHubDotComRepository(ctx, repoName); err != nil {
t.Fatal(err)
}
if !calledRepoLookup {
Expand Down
20 changes: 15 additions & 5 deletions cmd/frontend/backend/repos_vcs.go
View file
Open in desktop
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/sourcegraph/sourcegraph/cmd/frontend/envvar"
"github.com/sourcegraph/sourcegraph/cmd/frontend/types"
"github.com/sourcegraph/sourcegraph/pkg/api"
"github.com/sourcegraph/sourcegraph/pkg/extsvc/github"
"github.com/sourcegraph/sourcegraph/pkg/extsvc/gitlab"
"github.com/sourcegraph/sourcegraph/pkg/gitserver"
"github.com/sourcegraph/sourcegraph/pkg/repoupdater"
"github.com/sourcegraph/sourcegraph/pkg/repoupdater/protocol"
Expand All @@ -23,7 +25,11 @@ import (
// value), those operations will fail. This occurs when the repository isn't cloned on gitserver or
// when an update is needed (eg in ResolveRevision).
func CachedGitRepo(ctx context.Context, repo *types.Repo) (*gitserver.Repo, error) {
r, err := quickGitserverRepo(ctx, repo.Name)
var serviceType string
if repo.ExternalRepo != nil {
serviceType = repo.ExternalRepo.ServiceType
}
r, err := quickGitserverRepo(ctx, repo.Name, serviceType)
if err != nil {
return nil, err
}
Expand All @@ -36,7 +42,11 @@ func CachedGitRepo(ctx context.Context, repo *types.Repo) (*gitserver.Repo, erro
// GitRepo returns a handle to the Git repository with the up-to-date (as of the time of this call)
// remote URL. See CachedGitRepo for when this is necessary vs. unnecessary.
func GitRepo(ctx context.Context, repo *types.Repo) (gitserver.Repo, error) {
gitserverRepo, err := quickGitserverRepo(ctx, repo.Name)
var serviceType string
if repo.ExternalRepo != nil {
serviceType = repo.ExternalRepo.ServiceType
}
gitserverRepo, err := quickGitserverRepo(ctx, repo.Name, serviceType)
if err != nil {
return gitserver.Repo{Name: repo.Name}, err
}
Expand All @@ -57,7 +67,7 @@ func GitRepo(ctx context.Context, repo *types.Repo) (gitserver.Repo, error) {
return gitserver.Repo{Name: result.Repo.Name, URL: result.Repo.VCS.URL}, nil
}

func quickGitserverRepo(ctx context.Context, repo api.RepoName) (*gitserver.Repo, error) {
func quickGitserverRepo(ctx context.Context, repo api.RepoName, serviceType string) (*gitserver.Repo, error) {
// If it is possible to 100% correctly determine it statically, use a fast path. This is
// used to avoid a RepoLookup call for public GitHub.com and GitLab.com repositories
// (especially on Sourcegraph.com), which reduces rate limit pressure significantly.
Expand All @@ -67,9 +77,9 @@ func quickGitserverRepo(ctx context.Context, repo api.RepoName) (*gitserver.Repo
lowerRepo := strings.ToLower(string(repo))
var hasToken func(context.Context) (bool, error)
switch {
case strings.HasPrefix(lowerRepo, "github.com/"):
case serviceType == github.ServiceType && strings.HasPrefix(lowerRepo, "github.com/"):
hasToken = hasGitHubDotComToken
case strings.HasPrefix(lowerRepo, "gitlab.com/"):
case serviceType == gitlab.ServiceType && strings.HasPrefix(lowerRepo, "gitlab.com/"):
hasToken = hasGitLabDotComToken
default:
return nil, nil
Expand Down