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

Fix repository sync aborts after a signle repo failed #651

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions server/api/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func GetRepos(c *gin.Context) {
all, _ := strconv.ParseBool(c.Query("all"))
flush, _ := strconv.ParseBool(c.Query("flush"))

var flushError error
if flush || time.Unix(user.Synced, 0).Add(time.Hour*72).Before(time.Now()) {
log.Debug().Msgf("sync begin: %s", user.Login)
user.Synced = time.Now().Unix()
Expand All @@ -110,31 +111,40 @@ func GetRepos(c *gin.Context) {
Match: shared.NamespaceFilter(config.OwnersWhitelist),
}

if err := sync.Sync(c, user, server.Config.FlatPermissions); err != nil {
log.Debug().Msgf("sync error: %s: %s", user.Login, err)
flushError = sync.Sync(c, user, server.Config.FlatPermissions)
if flushError != nil {
log.Debug().Msgf("sync error: %s: %s", user.Login, flushError)
} else {
log.Debug().Msgf("sync complete: %s", user.Login)
}
}

repos, err := _store.RepoList(user, true)
allRepos, err := _store.RepoList(user, true)
if err != nil {
c.String(500, "Error fetching repository list. %s", err)
return
}

var repos []*model.Repo
if all {
c.JSON(http.StatusOK, repos)
return
repos = allRepos
} else {
for _, repo := range allRepos {
if repo.IsActive {
repos = append(repos, repo)
}
}
}

var active []*model.Repo
for _, repo := range repos {
if repo.IsActive {
active = append(active, repo)
}
result := model.RepoList{
Repos: repos,
}

if flushError != nil {
result.Message = flushError.Error()
}
c.JSON(http.StatusOK, active)

c.JSON(http.StatusOK, result)
}

func PostToken(c *gin.Context) {
Expand Down
5 changes: 5 additions & 0 deletions server/model/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,8 @@ type RepoPatch struct {
Visibility *string `json:"visibility,omitempty"`
AllowPull *bool `json:"allow_pr,omitempty"`
}

type RepoList struct {
Message string `json:"message"`
Repos []*Repo `json:"repos"`
}
19 changes: 17 additions & 2 deletions server/shared/userSyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"fmt"
"time"

"github.com/rs/zerolog/log"

"github.com/woodpecker-ci/woodpecker/server/model"
"github.com/woodpecker-ci/woodpecker/server/remote"
"github.com/woodpecker-ci/woodpecker/server/store"
Expand Down Expand Up @@ -67,6 +69,7 @@ func (s *Syncer) Sync(ctx context.Context, user *model.User, flatPermissions boo
}

remoteRepos := make([]*model.Repo, 0, len(repos))
hasErrors := false
for _, repo := range repos {
if s.Match(repo) {
repo.Perm = &model.Perm{
Expand All @@ -86,7 +89,9 @@ func (s *Syncer) Sync(ctx context.Context, user *model.User, flatPermissions boo
} else {
remotePerm, err := s.Remote.Perm(ctx, user, repo.Owner, repo.Name)
if err != nil {
return fmt.Errorf("could not fetch permission of repo '%s': %v", repo.FullName, err)
log.Debug().Msgf("could not fetch permission of repo '%s': %v", repo.FullName, err)
hasErrors = true
continue
Comment on lines +92 to +94
Copy link
Member

Choose a reason for hiding this comment

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

As this code solves a somehow "different" problem, I would suggest to just log the error as debug and ignore it otherwise. So we just keep these 3 lines of code for now.

Suggested change
log.Debug().Msgf("could not fetch permission of repo '%s': %v", repo.FullName, err)
hasErrors = true
continue
// TODO: report errors again after #648 got solved
log.Debug().Msgf("could not fetch permission of repo '%s': %v", repo.FullName, err)
continue

}
repo.Perm.Pull = remotePerm.Pull
repo.Perm.Push = remotePerm.Push
Expand All @@ -97,6 +102,7 @@ func (s *Syncer) Sync(ctx context.Context, user *model.User, flatPermissions boo
}
}

// still store all successfully queried repositories
err = s.Store.RepoBatch(remoteRepos)
if err != nil {
return err
Expand All @@ -113,5 +119,14 @@ func (s *Syncer) Sync(ctx context.Context, user *model.User, flatPermissions boo
return nil
}

return s.Perms.PermFlush(user, unix)
err = s.Perms.PermFlush(user, unix)
if err != nil {
return err
}

if hasErrors {
return fmt.Errorf("failed to sync all repositories. See previous messages for details")
}

return nil
}
17 changes: 14 additions & 3 deletions web/src/lib/api/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
import ApiClient, { encodeQueryString } from './client';
import { Build, BuildFeed, BuildLog, BuildProc, Registry, Repo, RepoPermissions, RepoSettings, Secret } from './types';
import {
Build,
BuildFeed,
BuildLog,
BuildProc,
Registry,
Repo,
RepoList,
RepoPermissions,
RepoSettings,
Secret,
} from './types';

type RepoListOptions = {
all?: boolean;
flush?: boolean;
};
export default class WoodpeckerClient extends ApiClient {
getRepoList(opts?: RepoListOptions): Promise<Repo[]> {
getRepoList(opts?: RepoListOptions): Promise<RepoList> {
const query = encodeQueryString(opts);
return this._get(`/api/user/repos?${query}`) as Promise<Repo[]>;
return this._get(`/api/user/repos?${query}`) as Promise<RepoList>;
}

getRepo(owner: string, repo: string): Promise<Repo> {
Expand Down
8 changes: 8 additions & 0 deletions web/src/lib/api/types/repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,11 @@ export type RepoPermissions = {
admin: boolean;
synced: number;
};

export type RepoList = {
message: string | null;
// error message if any errors occurred

repos: Repo[];
// list of the fetched repositories
};
4 changes: 2 additions & 2 deletions web/src/store/repos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export default defineStore({
this.repos[repoSlug(repo)] = repo;
},
async loadRepos() {
const repos = await apiClient.getRepoList();
repos.forEach((repo) => {
const result = await apiClient.getRepoList();
result.repos.forEach((repo) => {
this.repos[repoSlug(repo.owner, repo.name)] = repo;
});
},
Expand Down
18 changes: 15 additions & 3 deletions web/src/views/RepoAdd.vue
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,25 @@ export default defineComponent({
const { searchedRepos } = useRepoSearch(repos, search);

onMounted(async () => {
repos.value = await apiClient.getRepoList({ all: true });
const result = await apiClient.getRepoList({ all: true });
repos.value = result.repos;

if (result.message) {
notifications.notify({ title: result.message, type: 'error' });
}
});

const { doSubmit: reloadRepos, isLoading: isReloadingRepos } = useAsyncAction(async () => {
const result = await apiClient.getRepoList({ all: true, flush: true });

repos.value = undefined;
repos.value = await apiClient.getRepoList({ all: true, flush: true });
notifications.notify({ title: 'Repository list reloaded', type: 'success' });
repos.value = result.repos;

if (result.message) {
notifications.notify({ title: result.message, type: 'error' });
} else {
notifications.notify({ title: 'Repository list reloaded', type: 'success' });
}
});

const { doSubmit: activateRepo, isLoading: isActivatingRepo } = useAsyncAction(async (repo: Repo) => {
Expand Down
8 changes: 4 additions & 4 deletions woodpecker-go/woodpecker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,19 @@ func (c *client) Repo(owner string, name string) (*Repo, error) {
// RepoList returns a list of all repositories to which
// the user has explicit access in the host system.
func (c *client) RepoList() ([]*Repo, error) {
var out []*Repo
var out RepoList
uri := fmt.Sprintf(pathRepos, c.addr)
err := c.get(uri, &out)
return out, err
return out.Repos, err
}

// RepoListOpts returns a list of all repositories to which
// the user has explicit access in the host system.
func (c *client) RepoListOpts(sync, all bool) ([]*Repo, error) {
var out []*Repo
var out RepoList
uri := fmt.Sprintf(pathRepos+"?flush=%v&all=%v", c.addr, sync, all)
err := c.get(uri, &out)
return out, err
return out.Repos, err
}

// RepoPost activates a repository.
Expand Down
6 changes: 6 additions & 0 deletions woodpecker-go/woodpecker/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ type (
BuildCounter *int `json:"build_counter,omitempty"`
}

// RepoList defines the response type of the repos request
RepoList struct {
Message string `json:"message"`
Repos []*Repo `json:"repos"`
}

// Build defines a build object.
Build struct {
ID int64 `json:"id"`
Expand Down