Fix gitea API call for triggering pipelines #50
Conversation
|
Thanks for taking the time to send a pull request. Upon review I do not believe we need to add a new method for FindCommit(ctx context.Context, repo, ref string) (*Commit, *Response, error) {
+ if strings.HasPrefix(ref, "refs/") {
+ // new API call
+ }
path := fmt.Sprintf("api/v1/repos/%s/git/commits/%s", repo, ref)
out := new(commitInfo)
res, err := s.client.do(ctx, "GET", path, nil, out)
return convertCommitInfo(out), res, err
} |
scm/driver/gitea/git.go
Outdated
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| sha := out[0].Object.Sha |
There was a problem hiding this comment.
this code is subject to a panic if the index is out of bounds, so we should probably check the length of the slice just to be certain.
This indeed makes more sense, I have pushed a new fix, also checking the slice length as you remarked. |
|
UPDATE: I'm wrong, we haven't implemented that. But I think it's better for gitea to implement it so that there is only one request/reponse. |
|
go-gitea/gitea#10915 got merged ... |
|
|
I have updated the code accordingly! |
| @@ -24,7 +25,11 @@ func (s *gitService) FindBranch(ctx context.Context, repo, name string) (*scm.Re | |||
| } | |||
|
|
|||
| func (s *gitService) FindCommit(ctx context.Context, repo, ref string) (*scm.Commit, *scm.Response, error) { | |||
| path := fmt.Sprintf("api/v1/repos/%s/git/commits/%s", repo, ref) | |||
| ref = strings.TrimPrefix(ref, "refs/") | |||
There was a problem hiding this comment.
I noticed this code trims refs/. Is it correct that refs/heads/master should be sent to the Gitea API as heads/master? I just wanted to confirm ...
There was a problem hiding this comment.
With the latest gitea image it was necessary yes. This is the new API call go-gitea/gitea#10915. It would of course be nice if someone else could verify it also works for him/her.
There was a problem hiding this comment.
Hello @bradrydzewski , we spoke about this issue (failing cron builds on newer Gitea) in Drone's Gitter room last Friday, could I ask what is the current status? :)
There was a problem hiding this comment.
refs/heads/master works fine, trimming is unnecessary. The new API call behaves just like previous one, it's just a cosmetic change.
|
API in Gitea has been changed back to not be breaking, so it should work again as it was but now for both commit sha hash and ref |
|
The changes made by this PR are now unnecessary. As for the original issue this PR tried to resolve (triggering pipeline via branch), the issue was never with Gitea API - it allows (and always did) passing ref or SHA commit. We have however noticed potential reason why the problem exists - the |
#61 with the latest gitea build fixed the issue for me, closing this PR now. |
feat: add function to get installations for an organisation
@bradrydzewski
Solution to the problem from this issue.
All that remains is to change drone/service/commit/commit.go line 90 to
commit, _, err := s.client.Git.FindCommitByRef(ctx, repo, ref)to actually implement it.Let me know if I need to make any changes regarding code style etc.