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

repos: Don't cancel context on unauth / forbidden / accountsuspended#22947

Merged
tsenart merged 1 commit intomainfrom
streaming-repos-syncer-fix-deletion-on-errors
Jul 17, 2021
Merged

repos: Don't cancel context on unauth / forbidden / accountsuspended#22947
tsenart merged 1 commit intomainfrom
streaming-repos-syncer-fix-deletion-on-errors

Conversation

@tsenart
Copy link
Copy Markdown
Contributor

@tsenart tsenart commented Jul 17, 2021

Cancelling that context eagerly means all follow up operations that use it will fail with context: canceled

This also fixes the tests to actually catch this bug. Saw this in https://sourcegraph.com/-/debug/proxies/repo-updater-10.164.5.144/debug/requests?fam=Store.DeleteExternalServiceReposNotIn&b=8&exp=1

Cancelling that context eagerly means all follow up operations that use
it will fail with context: canceled

This also fixes the tests to actually catch this bug.
@tsenart tsenart requested a review from ryanslade July 17, 2021 11:31
@tsenart tsenart enabled auto-merge (squash) July 17, 2021 11:34
@tsenart tsenart merged commit 258ffc8 into main Jul 17, 2021
@tsenart tsenart deleted the streaming-repos-syncer-fix-deletion-on-errors branch July 17, 2021 11:35
tsenart added a commit that referenced this pull request Jul 17, 2021
This commit makes the streaming syncer the default mode, while giving
admins the option to disable it by setting the env var to false.

We've been running this in production for about a week without any
major problems apart from a small bug that only affected sourcegraph.com
in practice (that was fixed in #22947).

Making it the default will benefit large customers already and allow us
to start deleting old code paths.
tsenart pushed a commit that referenced this pull request Jul 19, 2021
* repos: Make streaming syncer the default

This commit makes the streaming syncer the default mode, while giving
admins the option to disable it by setting the env var to false.

We've been running this in production for about a week without any
major problems apart from a small bug that only affected sourcegraph.com
in practice (that was fixed in #22947).

Making it the default will benefit large customers already and allow us
to start deleting old code paths.

* fixup! Update CHANGELOG
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant