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

Return 204 instead of 404 if no jobs waiting #9671

Closed
wants to merge 1 commit into from
Closed

Return 204 instead of 404 if no jobs waiting #9671

wants to merge 1 commit into from

Conversation

janten
Copy link
Contributor

@janten janten commented Sep 26, 2015

@stanhu
Copy link
Member

stanhu commented Sep 26, 2015

/cc: @ayufan

@janten
Copy link
Contributor Author

janten commented Oct 18, 2015

Any news on this?

@JulienBlancher
Copy link

+1 would be great :)

@ayufan
Copy link
Member

ayufan commented Nov 16, 2015

@janten @JulienBlancher I expect it to be resolved in 8.3.

@JulienBlancher
Copy link

Ok nice thanks !

@stanhu
Copy link
Member

stanhu commented Dec 1, 2015

Should this be closed, @ayufan?

@ayufan
Copy link
Member

ayufan commented Dec 1, 2015

@stanhu Not yet, I want to test it and possibly merge in the upcoming release.

@janten
Copy link
Contributor Author

janten commented Dec 22, 2015

Any news on this? 8.3 still returns the 404 error.

@asziranyi
Copy link

@ayufan can you give us a status on this issue? Thanks.

@cadavre
Copy link

cadavre commented Apr 4, 2016

Any info? Why return error on proper check? A must to implement.

@Razer6
Copy link
Member

Razer6 commented Apr 6, 2016

/cc @rymai

@ayufan
Copy link
Member

ayufan commented Apr 6, 2016

@tmaczukin I think that we should be ready to merge this change. Could you verify what consequences it will have for runner older than 0.7, and also previous ruby runner? From what I think it will only lead to failure message printed in logs.

@tmaczukin
Copy link
Member

@ayufan OK, I will look on this.

@tmaczukin
Copy link
Member

@ayufan Old ruby runner was looking only for status code 201 (from some version also for 403) while receiving new builds. Any other status code was causing a message "nothing". So for old ruby runner there is no difference between 204 and 404.

New runner in go, before version 1.0.0, was looking for more status codes, but any other than 201 was causing only an warning/error message (and for 403 or clientError there is also false returned from the GetBuild method). 204 in case of versions older than 1.0.0 will be matched by default in the switch statement, and because of this the message will be an Error 'Checking for builds... failed'. But return of the method will be the same as for 404 code: return nil, true.

Since version 1.0.0 status code 204 is caught in parallel with 404 in the switch statement.

@connorshea
Copy link
Contributor

Thanks for the contribution to GitLab! Unfortunately we're no longer accepting Pull Requests through the GitHub project.

If you'd like to get this merged, I encourage you to open this as a Merge Request on the GitLab.com project. Feel free to ping me (@connorshea) and I'll get the right person assigned to it.

Apologies for the troubles, and thanks again for helping improve the project! ❤️

@connorshea connorshea closed this Aug 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
9 participants