-
Notifications
You must be signed in to change notification settings - Fork 177
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
fix: add capability to update existing secret #3551
fix: add capability to update existing secret #3551
Conversation
Hi @olavtar. Thanks for your PR. I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
f07ce52
to
5789e7e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3551 +/- ##
==========================================
- Coverage 85.55% 85.40% -0.15%
==========================================
Files 1373 1378 +5
Lines 31195 31315 +120
Branches 8736 8761 +25
==========================================
+ Hits 26689 26746 +57
- Misses 4506 4569 +63 see 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
/ok-to-test |
/lgtm |
const { coreV1Api } = fastify.kube; | ||
const { namespace } = fastify.kube; |
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.
Nit, you should really put these in the same line
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
… is created or retrieved Signed-off-by: Olga Lavtar <olavtar@redhat.com>
ba094d3
to
dff66d7
Compare
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
dff66d7
to
5b180e7
Compare
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
/lgtm |
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 think this change is needed, please do it in a future PR.
Nothing else is wrong with this PR so I'm going to approve it; but it definitely could use with some wording update.
@@ -68,10 +64,9 @@ export const createNIMAccount = async ( | |||
|
|||
export const createNIMSecret = async ( |
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.
This needs to be named differently
export const createNIMSecret = async ( | |
export const manageNIMSecret = async ( |
Or something else... it's not just creating it anymore. It's confusing to read the code.
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.
Ok, will update in the future PR. Thanks.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
https://issues.redhat.com/browse/NVPE-77
Description
Added the ability to update the existing secret with the new API key.
How Has This Been Tested?
Test Impact
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main