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

CASMCMS-8978: Fix broken _matches_filter call in patch_v2_components_dict #133

Merged
merged 2 commits into from
May 7, 2024

Conversation

mharding-hpe
Copy link
Contributor

@mharding-hpe mharding-hpe commented May 7, 2024

The fix for CASMTRIAGE-5974 modified the _matches_filter function in src/server/cray/cfs/api/controllers/components.py, adding the id_list argument. (PR: #82). However, it overlooked the call to this function in the patch_v2_components_dict function. So now that function raises an exception when it tries to make that call, due to the omitted argument.

Originally, this PR just modified the call so that it included all of the required arguments. But while that did allow my PATCH requests to succeed, I noticed that they were modifying more components than they should be. This led me to discover that there is a bug in the status filtering code for v2 -- specifically, the _set_status function is not used, and so the filter is not being correctly applied. This PR corrects that as well.

Backport for CSM 1.5:
#134

Partial backport for CSM 1.4:
#135
(the original bug does not exist in CSM 1.4, but the status filtering issue does)

I tested the CSM 1.5 changes on drax.
I tested the CSM 1.4 changes on wasp.

@mharding-hpe mharding-hpe changed the title CASMCMS-8978: Add missing id_list arg to _matches_filter call in patch_v2_components_dict CASMCMS-8978: Fix broken _matches_filter call in patch_v2_components_dict May 7, 2024
@mharding-hpe mharding-hpe merged commit 4c67c91 into master May 7, 2024
6 of 7 checks passed
@mharding-hpe mharding-hpe deleted the casmcms-8978 branch May 7, 2024 19:41
mharding-hpe added a commit that referenced this pull request May 7, 2024
…715110936

[chore] master -> develop from PR #133 (casmcms-8978)
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.

2 participants