search

fabric8-services / fabric8-jenkins-proxy

Apache License 2.0
1 stars 16 forks source link

Issue #133 NotFound should be an error #204

Closed kishansagathiya closed 6 years ago

kishansagathiya commented 6 years ago

Now if namespace is not found info api will through an error

Fixes #133

kishansagathiya commented 6 years ago

@chmouel WDYT?

chmouel commented 6 years ago

Can you please word a bit more the issue and want you try to achieve in the commit, i don't quite understand it,

kishansagathiya commented 6 years ago

@chmouel Should we throw an error if namespace passed with info api is not found? So, far we weren't. This PR changes that.

this would happen when we run GET /api/info/$SOME_NAMESPACE_THAT DOESNT_EXIST

chmouel commented 6 years ago

Sure but please update the title of this commit/issue then and description,

kishansagathiya commented 6 years ago

Let's keep this one on HOLD. There are places where if notFound, it records new statistics considering it as a new user

kishansagathiya commented 6 years ago

Ok. Turns out notFound should not throw an error. NotFound means it is not found in the database. If the namespace is not in the database, it will add the namespace to the database. It is not supposed to be an error. Let's close this PR and the issue as well. @chmouel WDYT?

Some uses of notFound https://github.com/fabric8-services/fabric8-jenkins-proxy/blob/8338ca57095f76d445dbfc6cb02fbe44052dbf84/internal/api/api.go#L40 https://github.com/fabric8-services/fabric8-jenkins-proxy/blob/8338ca57095f76d445dbfc6cb02fbe44052dbf84/internal/proxy/proxy.go#L615

chmouel commented 6 years ago

You know better this code than me :) if you tell me this can be closed, so be it, but yeah from your explanation this should not be indeed an error.

kishansagathiya commented 6 years ago

If you look at this you will find that if notFound, it creates newStatistics

https://github.com/fabric8-services/fabric8-jenkins-proxy/blob/8338ca57095f76d445dbfc6cb02fbe44052dbf84/internal/proxy/proxy.go#L615-L633