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.

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

repo-updater: Explicity disallow external services from syncing on Cloud #17463

Closed
wants to merge 2 commits into from

Conversation

ryanslade
Copy link
Contributor

@ryanslade ryanslade commented Jan 20, 2021

Instead of ignoring all site level external services from syncing on
Cloud we instead limit it to only exclude our "global" external services
as these are the ones we expect to have very large numbers of repos.

Might fix: #17424

Instead of ignoring all site level external services from syncing on
Cloud we instead limit it to only exclude our "global" external services
as these are the ones we expect to have very large numbers of repos.
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jan 20, 2021

Notifying subscribers in CODENOTIFY files for diff 0197ed6...bf439d2.

Notify File(s)
@asdine internal/repos/store.go
internal/repos/store_test.go

@codecov
Copy link

codecov bot commented Jan 20, 2021

Codecov Report

Merging #17463 (bf439d2) into main (561c07f) will increase coverage by 0.00%.
The diff coverage is 85.71%.

@@           Coverage Diff           @@
##             main   #17463   +/-   ##
=======================================
  Coverage   51.93%   51.94%           
=======================================
  Files        1715     1715           
  Lines       85198    85200    +2     
  Branches     7683     7683           
=======================================
+ Hits        44251    44255    +4     
+ Misses      37047    37043    -4     
- Partials     3900     3902    +2     
Flag Coverage Δ *Carryforward flag
go 50.94% <85.71%> (+<0.01%) ⬆️
integration 30.69% <ø> (ø) Carriedforward from 8e51e56
storybook 30.34% <ø> (ø) Carriedforward from 8e51e56
typescript 54.36% <ø> (ø) Carriedforward from 8e51e56
unit 34.72% <ø> (ø) Carriedforward from 8e51e56

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
internal/repos/syncer.go 77.74% <0.00%> (ø)
internal/repos/store.go 86.66% <100.00%> (+0.06%) ⬆️
...nal/campaigns/resolvers/changeset_apply_preview.go 65.92% <0.00%> (+1.48%) ⬆️

@@ -162,10 +164,12 @@ func Main(enterpriseInit EnterpriseInit) {
case *schema.GitHubConnection:
if strings.HasPrefix(c.Url, "https://github.com") && c.Token != "" && c.CloudGlobal {
server.GithubDotComSource, err = repos.NewGithubSource(e, cf)
excludeFromSyncing = append(excludeFromSyncing, e.ID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we could do this filtering in the database instead? Like adding a column to the external services table and filtering them on l149

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that'd be cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this when I originally added the "cloud global" idea. The problem is then that we need to manually update the database in order to make changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it only applies to GitHub and GitLab

@ryanslade
Copy link
Contributor Author

Hmm, I also just realised that we probably also want to index all synced repos.

@ryanslade
Copy link
Contributor Author

Closing in favour of #17472

@ryanslade ryanslade closed this Jan 20, 2021
@ryanslade ryanslade deleted the explicit-sync-opt-out branch June 21, 2021 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sourcegraph.com not indexing repos added by org
4 participants