[API] GetRelease by tag only return release #14397

cameronbraid · 2021-01-19T05:04:04Z

get release by tag should filter out tag releases to be consistent with list releases and get by id

There is a inconsistency between the 'list releases for repo' and 'get release by tag'.

The former filters out any release that is_tag=true where as the later includes them.

So there is no consistent way to identify if a release exists for a given tag name.

If I iterate the 'list releases for repo' results it wont be there. If I use 'get release by tag' it will be there but won't have any fields like title/notes/commitssh

…h list releases and get by id

routers/api/v1/repo/release_tags.go

cameronbraid · 2021-01-19T05:21:26Z

Can I also backport this to release/v1.13 ?

CirnoT · 2021-01-19T05:27:31Z

Makes it even more inconsistent due to DELETE being used to remove tag.

Your solution is sound however it would require:

Removing DELETE on /repos/{owner}/{repo}/releases/tags/{tag} (having endpoint that removes tag grouped under release seems counter-intuitive anyway, release is not a tag nor can release have more than one tag)
Adding GET for /repos/{owner}/{repo}/tags/{tag} to fetch specific tag
Adding DELETE for /repos/{owner}/{repo}/tags/{tag} to remove specific tag

codecov-io · 2021-01-19T05:36:51Z

Codecov Report

Merging #14397 (76da070) into master (0c0445c) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #14397      +/-   ##
==========================================
+ Coverage   41.84%   41.86%   +0.01%     
==========================================
  Files         744      744              
  Lines       79779    79782       +3     
==========================================
+ Hits        33387    33400      +13     
+ Misses      40874    40866       -8     
+ Partials     5518     5516       -2

Impacted Files	Coverage Δ
routers/api/v1/repo/release_tags.go	`48.38% <0.00%> (-5.19%)`	⬇️
services/pull/check.go	`48.52% <0.00%> (-0.74%)`	⬇️
services/pull/pull.go	`42.64% <0.00%> (+0.49%)`	⬆️
models/repo_list.go	`78.76% <0.00%> (+0.88%)`	⬆️
modules/queue/workerpool.go	`60.81% <0.00%> (+2.04%)`	⬆️
modules/git/tree_entry_nogogit.go	`93.75% <0.00%> (+6.25%)`	⬆️
modules/util/timer.go	`85.71% <0.00%> (+42.85%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c0445c...76da070. Read the comment docs.

cameronbraid · 2021-01-19T06:10:28Z

Not sure what you want conceptually for how gitea wants release/tags to function

From my limited understanding, a tag can exist without a release - however there is still a row in the release table for said tag

A release can be created for a tag which upgrades the row in the release table to be is_tag = false and fills on extra information

I assumes that the GET/DELETE on /repos/{owner}/{repo}/releases/tags/{tag} were alternative endpoints for GET/DELETE on /repos/{owner}/{repo}/releases/{id} but doing the lookups buy tag name instead of release id

CirnoT · 2021-01-19T21:26:14Z

I assumes that the GET/DELETE on /repos/{owner}/{repo}/releases/tags/{tag} were alternative endpoints for GET/DELETE on /repos/{owner}/{repo}/releases/{id} but doing the lookups buy tag name instead of release id

No, the DELETE endpoint works only if the release is is_tag=true and is used to remove tag via git itself:

gitea/routers/api/v1/repo/release_tags.go

Line 116 in 76da070

    
           if err := releaseservice.DeleteReleaseByID(release.ID, ctx.User, true); err != nil {

gitea/services/release/release.go

Lines 149 to 155 in 8c086ba

    
           if delTag { 
        
           	if stdout, err := git.NewCommand("tag", "-d", rel.TagName). 
        
           		SetDescription(fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID)). 
        
           		RunInDir(repo.RepoPath()); err != nil && !strings.Contains(err.Error(), "not found") { 
        
           		log.Error("DeleteReleaseByID (git tag -d): %d in %v Failed:\nStdout: %s\nError: %v", rel.ID, repo, stdout, err) 
        
           		return fmt.Errorf("git tag -d: %v", err) 
        
           	}

By changing how GET endpoint works, you make it inconsistent with how DELETE behaves (note it can't be used to delete release!)

gitea/routers/api/v1/repo/release_tags.go

Lines 111 to 113 in 76da070

    
           if !release.IsTag { 
        
           	ctx.Error(http.StatusConflict, "IsTag", errors.New("a tag attached to a release cannot be deleted directly")) 
        
           	return

6543

this is a breaking change!
and should at first be considdered carefully!

6543 · 2021-01-20T00:49:45Z

was added at #12932 related: #13358

routers/api/v1/repo/release_tags.go

6543 · 2021-01-20T01:09:43Z

Summary of my review:

the #13358 is not jet been released ....

so we could make DELETE /repos/{owner}/{repo}/releases/tags/{tag} to be used as "delete release by tag"
we then have to add a delete git tag endpoint

I would suggest DELETE /repos/{owner}/{repo}/tags/{tag}
and add the check suggested by this pull ...

@cameronbraid ☝️ can you ajust this pull at least to the last one ... then I'll file a pull up myselve ... or you could go for the whole thing ;)

johanvdw · 2021-01-25T22:11:34Z

It seems the github api is also filtering out tags without release:
https://api.github.com/repos/go-gitea/gitea/releases/tags/v0.9.9
vs
https://api.github.com/repos/go-gitea/gitea/releases/tags/v1.0.0

6543 · 2021-02-04T01:54:23Z

I'll adjust this pull & send a follow up ...

6543 · 2021-02-04T03:11:21Z

🚀

* master: (22 commits) Add support for ref parameter to get raw file API (go-gitea#14602) Fixed irritating error message related to go version (go-gitea#14611) Use OldRef instead of CommitSHA for DeleteBranch comments (go-gitea#14604) Add information on how to build statically (go-gitea#14594) [skip ci] Updated translations via Crowdin Exclude the current dump file from the dump (go-gitea#14606) Remove spurious DataAsync Error logging (go-gitea#14599) [API] Add delete release by tag & fix unreleased inconsistency (go-gitea#14563) Fix rate limit bug when downloading assets on migrating from github (go-gitea#14564) [API] Add affected files of commits to commit struct (go-gitea#14579) [skip ci] Updated licenses and gitignores Fix locale init (go-gitea#14582) Add Content-Length header to HEAD requests (go-gitea#14542) Honor REGISTER_MANUAL_CONFIRM when doing openid registration (go-gitea#14548) Fix lfs file viewer (go-gitea#14568) Fix typo in generate-emoji.go (go-gitea#14570) Fix bug about ListOptions and stars/watchers pagnation (go-gitea#14556) Fix gpg key deletion (go-gitea#14561) [API] GetRelease by tag only return release (go-gitea#14397) Reduce data races (go-gitea#14549) ...

get release by tag should filter out tag releases to be consisten wit…

7882390

…h list releases and get by id

lafriks added the type/bug label Jan 19, 2021

lafriks added this to the 1.14.0 milestone Jan 19, 2021

lafriks reviewed Jan 19, 2021

View reviewed changes

routers/api/v1/repo/release_tags.go Outdated Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 19, 2021

remove RepoID test

876d7b1

lafriks approved these changes Jan 19, 2021

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 19, 2021

a1012112796 approved these changes Jan 19, 2021

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 19, 2021

Merge branch 'master' into pr-2edf7af9-56b4-48f6-ace2-decb1a5eaa3b

76da070

6543 requested changes Jan 20, 2021

View reviewed changes

6543 added modifies/api This PR adds API routes or modifies them pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! labels Jan 20, 2021

6543 reviewed Jan 20, 2021

View reviewed changes

routers/api/v1/repo/release_tags.go Outdated Show resolved Hide resolved

simplify & enhance

d3402e5

6543 changed the title ~~get release by tag should filter out tag releases~~ [API] GetRelease by tag only return release Feb 4, 2021

6543 approved these changes Feb 4, 2021

View reviewed changes

Merge branch 'master' into pr-2edf7af9-56b4-48f6-ace2-decb1a5eaa3b

1fc3ef4

6543 mentioned this pull request Feb 4, 2021

[API] Add delete release by tag & fix unreleased inconsistency #14563

Merged

1 task

fix test

e6ceff9

6543 merged commit 3c965c3 into go-gitea:master Feb 4, 2021

go-gitea locked and limited conversation to collaborators Mar 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[API] GetRelease by tag only return release #14397

[API] GetRelease by tag only return release #14397

Uh oh!

cameronbraid commented Jan 19, 2021

Uh oh!

Uh oh!

cameronbraid commented Jan 19, 2021

Uh oh!

CirnoT commented Jan 19, 2021

Uh oh!

codecov-io commented Jan 19, 2021 •

edited

Loading

Uh oh!

cameronbraid commented Jan 19, 2021

Uh oh!

CirnoT commented Jan 19, 2021 •

edited

Loading

Uh oh!

6543 left a comment

Uh oh!

6543 commented Jan 20, 2021

Uh oh!

Uh oh!

6543 commented Jan 20, 2021 •

edited

Loading

Uh oh!

johanvdw commented Jan 25, 2021

Uh oh!

6543 commented Feb 4, 2021

Uh oh!

6543 commented Feb 4, 2021

Uh oh!

Uh oh!

Uh oh!

[API] GetRelease by tag only return release #14397

[API] GetRelease by tag only return release #14397

Uh oh!

Conversation

cameronbraid commented Jan 19, 2021

Uh oh!

Uh oh!

cameronbraid commented Jan 19, 2021

Uh oh!

CirnoT commented Jan 19, 2021

Uh oh!

codecov-io commented Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cameronbraid commented Jan 19, 2021

Uh oh!

CirnoT commented Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

6543 left a comment

Choose a reason for hiding this comment

Uh oh!

6543 commented Jan 20, 2021

Uh oh!

Uh oh!

6543 commented Jan 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johanvdw commented Jan 25, 2021

Uh oh!

6543 commented Feb 4, 2021

Uh oh!

6543 commented Feb 4, 2021

Uh oh!

Uh oh!

codecov-io commented Jan 19, 2021 •

edited

Loading

CirnoT commented Jan 19, 2021 •

edited

Loading

6543 commented Jan 20, 2021 •

edited

Loading