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

[YUNIKORN-781] Add queue properties to the response of /ws/v1/partition/{partition}/queues #291

Merged
merged 3 commits into from
Aug 5, 2021

Conversation

chia7712
Copy link
Contributor

@chia7712 chia7712 commented Aug 4, 2021

What is this PR for?

That helps us observe the "application.sort.policy" through REST APIs.

What type of PR is it?

  • - Bug Fix
  • - Improvement
  • - Feature
  • - Documentation
  • - Hot Fix
  • - Refactoring

Todos

  • - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-781

How should this be tested?

TestGetPartitionQueues

Screenshots (if appropriate)

Questions:

  • - The licenses files need update.
  • - There is breaking changes for older versions.
  • - It needs documentation.

@chia7712
Copy link
Contributor Author

chia7712 commented Aug 4, 2021

I will another PR to update the APIs doc (https://yunikorn.apache.org/docs/api/scheduler) after this PR is accepted

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #291 (51918ba) into master (5a1c19e) will increase coverage by 2.04%.
The diff coverage is 65.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #291      +/-   ##
==========================================
+ Coverage   63.46%   65.51%   +2.04%     
==========================================
  Files          60       60              
  Lines        5220     5709     +489     
==========================================
+ Hits         3313     3740     +427     
- Misses       1747     1780      +33     
- Partials      160      189      +29     
Impacted Files Coverage Δ
pkg/common/utils.go 77.50% <0.00%> (ø)
pkg/metrics/init.go 62.96% <ø> (ø)
pkg/scheduler/context.go 5.68% <0.00%> (-0.58%) ⬇️
pkg/scheduler/nodes_usage_monitor.go 0.00% <0.00%> (ø)
pkg/scheduler/objects/allocation.go 100.00% <ø> (ø)
pkg/webservice/webservice.go 32.25% <ø> (+19.35%) ⬆️
pkg/metrics/metrics_collector.go 63.33% <30.00%> (-10.58%) ⬇️
pkg/scheduler/partition_manager.go 20.00% <50.00%> (ø)
pkg/metrics/scheduler.go 60.79% <55.00%> (-0.79%) ⬇️
pkg/scheduler/objects/application.go 57.44% <65.44%> (+8.53%) ⬆️
... and 13 more

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 8126eae...51918ba. Read the comment docs.

@yangwwei
Copy link
Contributor

yangwwei commented Aug 4, 2021

@manirajv06 can u help to review this PR?

@yangwwei yangwwei changed the title YUNIKORN-781 add to response of /ws/v1/partition/{partition}/queues [YUNIKORN-781] add to response of /ws/v1/partition/{partition}/queues Aug 5, 2021
@yangwwei yangwwei changed the title [YUNIKORN-781] add to response of /ws/v1/partition/{partition}/queues [YUNIKORN-781] Add queue properties to the response of /ws/v1/partition/{partition}/queues Aug 5, 2021
Copy link
Contributor

@yangwwei yangwwei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks @manirajv06 is not available right now.
this is a straightforward fix, looks good to me, thank you @chia7712 !

@manirajv06
Copy link
Contributor

looks @manirajv06 is not available right now.
this is a straightforward fix, looks good to me, thank you @chia7712 !

Sorry for the delay. @chia7712 Can we modify TestGetPartitionQueuesHandler in handlers_test.go to cover this change as it would be more appropriate? Can we create a separate PR under this https://issues.apache.org/jira/browse/YUNIKORN-784 jira itself?

@chia7712
Copy link
Contributor Author

chia7712 commented Aug 6, 2021

Can we modify TestGetPartitionQueuesHandler in handlers_test.go to cover this change as it would be more appropriate?

sure

Can we create a separate PR under this https://issues.apache.org/jira/browse/YUNIKORN-784 jira itself?

Did you mean "YUNIKORN-781"? YUNIKORN-784 is used to modify website doc.

@chia7712
Copy link
Contributor Author

chia7712 commented Aug 6, 2021

Can we modify TestGetPartitionQueuesHandler in handlers_test.go to cover this change as it would be more appropriate?

#293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants