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-1124] Avoid passing empty nodeAttributes in UpdateNode request #390

Closed
wants to merge 1 commit into from

Conversation

manirajv06
Copy link
Contributor

What is this PR for?

Shim do not pass the following:

  1. Empty NodeAttributes map as part of UpdateRequest (for delete, restore & decommissioning actions).
  2. Removed OccupiedResource and labels from UpdateRequest (create action) as they are not required on the core side

What type of PR is it?

  • - Improvement

Todos

  • - Task

What is the Jira issue?

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

How should this be tested?

Screenshots (if appropriate)

Questions:

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

@manirajv06 manirajv06 self-assigned this Mar 17, 2022
@codecov
Copy link

codecov bot commented Mar 17, 2022

Codecov Report

Merging #390 (27804e5) into master (527f4d1) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #390      +/-   ##
==========================================
- Coverage   65.84%   65.82%   -0.03%     
==========================================
  Files          40       40              
  Lines        6230     6226       -4     
==========================================
- Hits         4102     4098       -4     
  Misses       1973     1973              
  Partials      155      155              
Impacted Files Coverage Δ
pkg/cache/node.go 90.06% <100.00%> (-0.07%) ⬇️
pkg/common/si_helper.go 99.33% <100.00%> (-0.02%) ⬇️

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 527f4d1...27804e5. Read the comment docs.

Copy link
Contributor

@wilfred-s wilfred-s left a comment

Choose a reason for hiding this comment

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

LGTM

I was not sure about OccupiedResource at first but the node passed in is a "new" node which will not have occupied set ever so it can be removed.

@wilfred-s wilfred-s closed this in 1fbf8c2 Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants