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

Enable new perms syncer by default#47783

Merged
0xnmn merged 5 commits into
mainfrom
naman/new-perms-syncer-release
Feb 17, 2023
Merged

Enable new perms syncer by default#47783
0xnmn merged 5 commits into
mainfrom
naman/new-perms-syncer-release

Conversation

@0xnmn

@0xnmn 0xnmn commented Feb 17, 2023

Copy link
Copy Markdown
Contributor

closes: https://github.com/sourcegraph/sourcegraph/issues/47160

This PR enables the new perms syncer by default.

Test plan

  • new perms syncer should be running by default. check for logs
  • create feature flag and set to false
  • old runSchedule & runSync should log
  • set feature flag to true
  • new perms syncer should run.

@0xnmn 0xnmn self-assigned this Feb 17, 2023
@cla-bot cla-bot Bot added the cla-signed label Feb 17, 2023
@github-actions

github-actions Bot commented Feb 17, 2023

Copy link
Copy Markdown
Contributor

❌ Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.
❓ When does the freeze end? The code freeze is active until 2023-02-22 at 23:59 UTC.

@sourcegraph-bot

sourcegraph-bot commented Feb 17, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 3337387...0d53bdb.

Notify File(s)
@indradhanush enterprise/cmd/repo-updater/internal/authz/perms_syncer.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker.go
@unknwon enterprise/cmd/repo-updater/internal/authz/perms_syncer.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker.go
internal/authz/permssync/permssync.go
internal/authz/permssync/permssync_test.go

sashaostrikov
sashaostrikov reviewed Feb 17, 2023
View reviewed changes

@sashaostrikov sashaostrikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's remove some logs and make some log messages more explicit.
Apart from that LGTM! 👍🏻

Comment thread CHANGELOG.md Outdated
Comment thread enterprise/cmd/frontend/worker/auth/perms_syncer_scheduler.go Outdated
Comment thread enterprise/cmd/repo-updater/internal/authz/perms_syncer.go Outdated
Comment thread enterprise/cmd/repo-updater/internal/authz/perms_syncer.go Outdated
Comment thread enterprise/cmd/repo-updater/internal/authz/perms_syncer.go Outdated
Comment thread internal/authz/permssync/permssync.go Outdated
Comment thread internal/authz/permssync/permssync_test.go
@0xnmn 0xnmn requested a review from sashaostrikov February 17, 2023 08:25
@0xnmn 0xnmn enabled auto-merge (squash) February 17, 2023 08:25
sashaostrikov
sashaostrikov approved these changes Feb 17, 2023
View reviewed changes

@sashaostrikov sashaostrikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two small fixes and we're good! 👍

Comment thread enterprise/cmd/repo-updater/internal/authz/perms_syncer.go
Comment thread enterprise/cmd/repo-updater/internal/authz/perms_syncer.go Outdated
mrnugget
mrnugget approved these changes Feb 17, 2023
View reviewed changes

@mrnugget mrnugget left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LET
'S
GO

Comment thread CHANGELOG.md
kopancek
kopancek approved these changes Feb 17, 2023
View reviewed changes
Comment thread CHANGELOG.md
@0xnmn 0xnmn merged commit b10572f into main Feb 17, 2023
@0xnmn 0xnmn deleted the naman/new-perms-syncer-release branch February 17, 2023 08:45
@0xnmn 0xnmn mentioned this pull request Feb 17, 2023
Merged
0xnmn referenced this pull request Feb 17, 2023
@0xnmn
fix change log grammar (#47786)
933b56a 
In response to:
https://github.com/sourcegraph/sourcegraph/pull/47783#discussion_r1109445804
## Test plan
...
@ggilmore ggilmore mentioned this pull request Mar 21, 2023
Closed
26 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@mrnugget mrnugget mrnugget approved these changes

@kopancek kopancek kopancek approved these changes

@sashaostrikov sashaostrikov sashaostrikov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@0xnmn 0xnmn

Labels

cla-signed i-acknowledge-this-goes-into-the-release

Projects

No open projects
IAM Team
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Set new perms syncer feature flag to true by default

5 participants

@0xnmn @sourcegraph-bot @mrnugget @kopancek @sashaostrikov