gNSI:Add sonic service client support for Credentialz#671
gNSI:Add sonic service client support for Credentialz#671niranjanivivek wants to merge 1 commit into
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Neha Das nehadas@google.com Signed-off-by: Niranjani Vivek <niranjaniv@google.com>
niranjanivivek
force-pushed
the
gnsi_credz_fe_pr_new
branch
from
90d5414 to
6bb9789
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@ndas7 Please check and merge |
| timeout := 300 | ||
| if deadline, ok := ctx.Deadline(); ok { | ||
| remaining := time.Until(deadline) | ||
| if remaining <= 0 { | ||
| return context.DeadlineExceeded | ||
| } | ||
| timeout = int(remaining.Seconds()) | ||
| if timeout > 10 { | ||
| timeout = 10 | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a typo here? Seems like it should be 300 in the nested if clause
There was a problem hiding this comment.
HI @Bojun-Feng , Thanks for pointing this.
Yes we think it is a typo error. So if timeout exceeds 300s, it can be reset.
Is it fine to change the lines 543 -545 as below.? Please suggest if you have any comments here.
if timeout > 300 {
timeout = 300
}
There was a problem hiding this comment.
Hi @ndas7 @Bojun-Feng . Please respond , so that I can update the code and submit for review.
There was a problem hiding this comment.
Hi @niranjanivivek, apologies for the delay. For some context, I noticed the discrepancy in the code since I usually paste code blocks and forget to update the value in nested conditions. However, I am not fully knowledgeable on the context. I believe @ndas7 needs to confirm whether we want to align the value to 10 or 300.
I would recommend a single source of truth in the code block cited for this comment. Define another variable named threshold / timeout_cap / something descriptive, then reuse it for the entire block so the hard coded number only appears one time. This should reduce similar issues in the future.
There was a problem hiding this comment.
Hi @niranjanivivek, apologies for the delay. For some context, I noticed the discrepancy in the code since I usually paste code blocks and forget to update the value in nested conditions. However, I am not fully knowledgeable on the context. I believe @ndas7 needs to confirm whether we want to align the value to 10 or 300.
I would recommend a single source of truth in the code block cited for this comment. Define another variable named
threshold/timeout_cap/ something descriptive, then reuse it for the entire block so the hard coded number only appears one time. This should reduce similar issues in the future.
@ndas7 Please suggest the correct timeout value here as requested by Bojun-Feng.
4 participants
Signed-off-by: Neha Das nehadas@google.com
This PR enables support for managing credentials used for console and SSH access authentication on target device.
HLD Link
Dependency Chain for Merge
Backend transformer PR : sonic-net/sonic-mgmt-common#203
Backend sonic-host-services PR: sonic-net/sonic-host-services#347
Frontend Base PR: #671 - Merge first
Frontend Incremental PR : #672