-
Notifications
You must be signed in to change notification settings - Fork 5
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
CRAYSAT-956: Replace CAPMC calls with PCS calls #154
Conversation
4f709d7
to
07a2bec
Compare
1dcda9f
to
c5dca24
Compare
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
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'm just leaving some comments based on a code review with @shivaprasad-metimath and team that we did this morning.
1c85a47
to
54a9b48
Compare
618f4a3
to
c4e0466
Compare
c4e0466
to
8e5fd10
Compare
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 haven't reviewed the whole PR, but I've found some issues in the new PCSClient
class that need to be addressed.
b700533
to
27c39de
Compare
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.
There are some functional issues that need to be fixed before you try to test this with sat swap blade
.
27c39de
to
2ba2985
Compare
e4af5af
to
6a424c5
Compare
11340c3
to
9be08a5
Compare
Test output:
Observations:During enable of blade, time out was observed; During the power on of the nodes, timeout was observed |
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.
One minor issue was discovered with PCSError
class. This issue has been present since the original PR was raised. Shiva and I just noticed in while doing a code review on a meeting. It should be easy to address.
Please do file a CRAYSAT Jira for us to look at this problem you noted as well:
And please mark it related to CRAYSAT-1623. I was expecting that Jira to have resolved this issue. |
sure Ryan, will have new one created and add a ref to the old ticket |
CRAYSAT-1818 is filed by Harold recently. Can we use this jira instead of filing new one. |
Let me take a look at it Prasanth, it is same error reported, seems like a documentation changes are added |
55f5984
to
6abc097
Compare
This commit implements a basic PCSClient class which is mostly compatible with the CAPMCClient class. Usage of the CAPMCClient class was replaced with the new PCSClient class.
6abc097
to
882a059
Compare
Prereq, xname_errs removal removing the unused func: do_nodes_power_off and its testcases
882a059
to
e749e33
Compare
Summary and Scope
Replace calls to the CAPMC API with equivalent calls to the PCS API.
I tried to keep the software API of
PCSClient
mostly compatible withCAPMCClient
, though there may be differences that aren't covered in the tests.This hasn't been tested on a real system with PCS installed yet.
Issues and Related PRs
Testing
Tested on:
Test description:
Run unit tests.
Pull Request Checklist