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

Sync RemoteRepository and RemoteOrganization in all VCS providers #7310

Merged
merged 4 commits into from Aug 27, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Jul 22, 2020

  • Refactor Service.sync to be unique for all the VCS providers
  • Make sync_repositories and sync_organizations to return repositories
    and (organizations, repositories) to allow deleting old instances where the
    user does not have more access
  • rename BitBucketService.sync_teams to BitBucketService.sync_organizations
    to keep our internal naming

With this commit, all VCS providers (GitHub, GitLab and BitBucket) will start
deleting old RemoteRepository instances (not only GitHub as it's currently
working) but also deleting old RemoteOrganization instances as well.

This is a step previous to #2759 and it's required for GitLab/BitBucket VCS SSO

Closes #2412

- Refactor `Service.sync` to be unique for all the VCS providers
- Make `sync_repositories` and `sync_organizations` to return `repositories`
  and `(organizations, repositories)` to allow deleting old instances where the
  user does not have more access
- rename `BitBucketService.sync_teams` to `BitBucketService.sync_organizations`
  to keep our internal naming

With this commit, all VCS providers (GitHub, GitLab and BitBucket) will start
deleting old RemoteRepository instances (not only GitHub as it's currently
working) but also deleting old RemoteOrganization instances as well.
@humitos humitos requested a review from a team Jul 22, 2020
humitos added 2 commits Aug 18, 2020
When fetching repositories and organizations always return a valid list.
If the user does not have repositories or organizations, a empty list is
returned. This allows the code to continue working properly in the further steps.
Copy link
Member

@ericholscher ericholscher left a comment

This looks good 👍 Should keep our DB more nicely cleaned up.

repos = self.sync_repositories()
organizations, organization_repos = self.sync_organizations()

# Delete RemoteRepository where the user doesn't have access anymore
Copy link
Member

@ericholscher ericholscher Aug 26, 2020

Choose a reason for hiding this comment

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

I wonder if we should run these delete tasks across all users as a one-time CLI command, to clean up our DB. There's likely a ton of these sitting around.

Copy link
Member Author

@humitos humitos Aug 27, 2020

Choose a reason for hiding this comment

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

This one-time command would require to sync all VCS account connected and delete all the old objects. That may be too much. We may want to limit the amount of accounts re-synced and run it several times by chunks. It will remove a lot of stale data.

However, as this cleanup is in the core of the sync action, all active accounts will be cleanup automatically when they login next time or, when they click the arrow from Import Project page.

@humitos
Copy link
Member Author

humitos commented Aug 27, 2020

I'm going to merge this since it's useful as is it. We can keep the conversation about re-sync all the accounts and make a final decision, tho.

@humitos humitos merged commit 487fa72 into master Aug 27, 2020
2 checks passed
@humitos humitos deleted the humitos/sync-remote-repositories-organizations branch Aug 27, 2020
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.

Deleted repository not detected
2 participants