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

Fetch repositories with remote ID if possible #1078

Merged
merged 60 commits into from
Sep 5, 2022

Conversation

qwerty287
Copy link
Contributor

@qwerty287 qwerty287 commented Aug 5, 2022

Use IDs of the forge to fetch repositories instead of their names and owner names. This improves handling of renamed and transferred repos.

TODO

  • try to support as many forges as possible
    • Gogs (no API)
    • Bitbucket Server
    • Coding (no API?)
  • update repo every time it is fetched or received from the forge
  • if repo remote IDs are not available, use owner / name to get it
  • handle redirections (redirect a renamed repo to its new path)
  • pull all repos once during migration to update ID (?) issue fixed by on-demand loading of remote IDs
  • handle redirections in web UI
  • improve handling of hooks after a repo was renamed (currently it checks for a redirection to the repo)
  • tests
  • UNIQUE constraint for remote IDs after migration shouldn't work (all repos have an empty string as remote ID)

close #854
close #648 partial
close https://codeberg.org/Codeberg-CI/feedback/issues/46

Possible follow-up PRs

  • apply the same scheme on everything fetched from the remote (currently only users)

@qwerty287 qwerty287 added enhancement improve existing features refactor delete or replace old code labels Aug 5, 2022
@qwerty287 qwerty287 added this to the 1.0.0 milestone Aug 5, 2022
server/model/repo.go Outdated Show resolved Hide resolved
@qwerty287 qwerty287 added the bug Something isn't working label Aug 5, 2022
server/store/datastore/repo.go Outdated Show resolved Hide resolved
server/api/repo.go Outdated Show resolved Hide resolved
@6543
Copy link
Member

6543 commented Sep 3, 2022

@qwerty287 what does represent an empty remote_id in backend. "0" or "" or both ? -> we need to handle these cases or make sure they are validated before inserted

@qwerty287
Copy link
Contributor Author

Both are valid as empty values. Validation sounds like a good idea👍
I need to look when I have time to fix the reviews.

@qwerty287
Copy link
Contributor Author

@6543 sorry, just have to ask why we need validation here. If we're inserting something with an empty ID (which will happen if you use the coding backend), why is this bad? We always have the full name as fallback, and the ID is just ignored then. I don't see an issue if there are empty IDs in the DB.

@6543
Copy link
Member

6543 commented Sep 4, 2022

no we just need only one place where a func can determine if a repo has an id or not.
-> single source of truth

server/model/repo.go Outdated Show resolved Hide resolved
@6543 6543 enabled auto-merge (squash) September 5, 2022 14:21
@6543 6543 merged commit 52d3652 into woodpecker-ci:master Sep 5, 2022
@jdoubleu
Copy link
Contributor

jdoubleu commented Sep 5, 2022

Just noticed, that this part has not been removed:

// TODO(648) remove when woodpecker understands nested repos
if strings.Count(repo.FullName, "/") > 1 {
    log.Debug().Msgf("Skipping nested repository %s for user %s, because they are not supported, yet (see #648).", repo.FullName, user.Login)
    continue
}

It was introduced by !656.

Is this correct?

@qwerty287
Copy link
Contributor Author

qwerty287 commented Sep 5, 2022

It is. This PR didn't intend to support nested repos. It is just one part that will help (but actually one of the most important ones). Maybe, everything you've to do is to remove these lines you mentioned, and it will work, but I don't think so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement improve existing features refactor delete or replace old code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename of repo in forge break woodpecker repos support nested repositories (GitLab)
5 participants