-
Notifications
You must be signed in to change notification settings - Fork 241
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
[YUNIKORN-2856] Remove redundant conditional #964
Conversation
Hi, thanks for your PR. Please resolve syntax error. |
pkg/webservice/handlers.go
Outdated
if len(unescapedQueueName) == 0 { | ||
app = partitionContext.GetApplication(application) | ||
} else { | ||
else { | ||
queueErr := validateQueue(unescapedQueueName) | ||
if queueErr != nil { | ||
buildJSONErrorResponse(w, queueErr.Error(), http.StatusBadRequest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also refactor, and add test to prove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean by this
running unit tests
"go" clean -testcache
"go" test ./... -race -tags deadlock -coverprofile=build/coverage.txt -covermode=atomic
github.com/apache/yunikorn-core/cmd/schedulerclient coverage: 0.0% of statements
github.com/apache/yunikorn-core/cmd/queueconfigchecker coverage: 0.0% of statements
github.com/apache/yunikorn-core/cmd/simplescheduler coverage: 0.0% of statements
ok github.com/apache/yunikorn-core/pkg/common 3.427s coverage: 74.1% of statements
ok github.com/apache/yunikorn-core/pkg/common/configs 3.376s coverage: 97.8% of statements
ok github.com/apache/yunikorn-core/pkg/common/resources 5.427s coverage: 100.0% of statements
? github.com/apache/yunikorn-core/pkg/handler [no test files]
github.com/apache/yunikorn-core/pkg/examples coverage: 0.0% of statements
github.com/apache/yunikorn-core/pkg/events/mock coverage: 0.0% of statements
ok github.com/apache/yunikorn-core/pkg/common/security 8.745s coverage: 96.8% of statements
ok github.com/apache/yunikorn-core/pkg/entrypoint 2.559s coverage: 82.1% of statements
github.com/apache/yunikorn-core/pkg/mock coverage: 0.0% of statements
ok github.com/apache/yunikorn-core/pkg/events 3.868s coverage: 97.4% of statements
? github.com/apache/yunikorn-core/pkg/rmproxy/rmevent [no test files]
github.com/apache/yunikorn-core/pkg/rmproxy coverage: 0.0% of statements
ok github.com/apache/yunikorn-core/pkg/locking 5.329s coverage: 51.1% of statements
? github.com/apache/yunikorn-core/pkg/scheduler/placement/types [no test files]
github.com/apache/yunikorn-core/pkg/webservice/dao coverage: 0.0% of statements
ok github.com/apache/yunikorn-core/pkg/log 25.208s coverage: 76.3% of statements
ok github.com/apache/yunikorn-core/pkg/metrics 8.137s coverage: 69.5% of statements
ok github.com/apache/yunikorn-core/pkg/metrics/history 4.552s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/plugins 5.033s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler 8.046s coverage: 68.7% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/objects 4.299s coverage: 84.9% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/objects/events 6.734s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/objects/template 5.430s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/placement 4.549s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/policies 7.382s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/tests 48.139s coverage: [no statements]
ok github.com/apache/yunikorn-core/pkg/scheduler/ugm 3.840s coverage: 94.5% of statements
ok github.com/apache/yunikorn-core/pkg/webservice 5.892s coverage: 87.0% of statements
"go" vet github.com/apache/yunikorn-core/pkg...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I fix the syntax by changing to this(aebbea2)
the results in the make test is this:
running unit tests
"go" clean -testcache
"go" test ./... -race -tags deadlock -coverprofile=build/coverage.txt -covermode=atomic
github.com/apache/yunikorn-core/cmd/queueconfigchecker coverage: 0.0% of statements
github.com/apache/yunikorn-core/cmd/schedulerclient coverage: 0.0% of statements
github.com/apache/yunikorn-core/cmd/simplescheduler coverage: 0.0% of statements
github.com/apache/yunikorn-core/pkg/events/mock coverage: 0.0% of statements
github.com/apache/yunikorn-core/pkg/examples coverage: 0.0% of statements
? github.com/apache/yunikorn-core/pkg/handler [no test files]
ok github.com/apache/yunikorn-core/pkg/common 3.070s coverage: 74.1% of statements
ok github.com/apache/yunikorn-core/pkg/common/configs 3.123s coverage: 97.8% of statements
ok github.com/apache/yunikorn-core/pkg/common/resources 4.936s coverage: 100.0% of statements
github.com/apache/yunikorn-core/pkg/mock coverage: 0.0% of statements
github.com/apache/yunikorn-core/pkg/rmproxy coverage: 0.0% of statements
? github.com/apache/yunikorn-core/pkg/rmproxy/rmevent [no test files]
ok github.com/apache/yunikorn-core/pkg/common/security 5.959s coverage: 96.8% of statements
ok github.com/apache/yunikorn-core/pkg/entrypoint 7.105s coverage: 82.1% of statements
ok github.com/apache/yunikorn-core/pkg/events 5.834s coverage: 97.4% of statements
ok github.com/apache/yunikorn-core/pkg/locking 8.267s coverage: 51.1% of statements
? github.com/apache/yunikorn-core/pkg/scheduler/placement/types [no test files]
github.com/apache/yunikorn-core/pkg/webservice/dao coverage: 0.0% of statements
ok github.com/apache/yunikorn-core/pkg/log 25.608s coverage: 76.3% of statements
ok github.com/apache/yunikorn-core/pkg/metrics 9.365s coverage: 69.5% of statements
ok github.com/apache/yunikorn-core/pkg/metrics/history 6.755s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/plugins 5.666s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler 10.554s coverage: 68.7% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/objects 7.591s coverage: 84.9% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/objects/events 6.293s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/objects/template 6.005s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/placement 5.608s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/policies 5.793s coverage: 100.0% of statements
ok github.com/apache/yunikorn-core/pkg/scheduler/tests 47.177s coverage: [no statements]
ok github.com/apache/yunikorn-core/pkg/scheduler/ugm 4.489s coverage: 94.5% of statements
ok github.com/apache/yunikorn-core/pkg/webservice 5.945s coverage: 87.0% of statements
"go" vet github.com/apache/yunikorn-core/pkg...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, there's two routes using the same handler |
What is this PR for?
Remove redundant of if function.
What type of PR is it?
What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-2856