Retry from the first provided password if none of the passwords work#16870
Retry from the first provided password if none of the passwords work#16870bingwang-ms merged 2 commits intosonic-net:masterfrom
Conversation
In the DeviceConnection class, if none of the passwords appear to work, then try again from the first provided password. The reason for this is if a a device initially needs some password specified in the alternate passwords list, but then later needs to use some earlier-specified password (because of some config change), then connection attempts will fail until a new DeviceConnection object is instantiated. Instead, work around that by trying the passwords in a loop (i.e. from the beginning again). This also means that the class doesn't really need to keep track of what password might be the "primary" password and what password might be alternates. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| self.alt_password_index = 0 | ||
| self.passwords = [password] | ||
| if alt_password: | ||
| self.passwords += alt_password |
There was a problem hiding this comment.
$ python
Python 3.8.10 (default, Jan 17 2025, 14:40:23)
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>
>>>
>>> s = ["pass"]
>>> s += "pass2"
>>> s
['pass', 'p', 'a', 's', 's', '2']
>>>
I don't think this is what you need. Please check?
There was a problem hiding this comment.
alt_password is expected to be a list of strings.
The new behavior is the same as the old behavior; if someone passes in just a string, then each individual character of that string will get used as the password. I can add a check/handling here to accept only lists, or if a string is passed in, either error out or wrap it into a list.
| self.alt_password_index += 1 | ||
| if len(self.passwords) > 1: | ||
| # attempt retry with another password | ||
| self.password_index = (self.password_index + 1) % len(self.passwords) |
There was a problem hiding this comment.
Where will this be retried from and what's the limit of retry?
There was a problem hiding this comment.
There's a @retry decorator for this function that retries 4 times upon getting an AuthenticationException
There was a problem hiding this comment.
I see. The retry count should be a function of len(passwords)? So that your fix can take effect - every password can be tried at least twice?
There was a problem hiding this comment.
Ideally yes, but these variables are not defined when the @retry decorator is evaluated, so it can't be used. It actually might be a bit easier to do this without using the @retry decorator and doing the retry logic ourselves.
vaibhavhd
left a comment
vaibhavhd
left a comment
There was a problem hiding this comment.
Approved with non blocking suggestions
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…onic-net#16870) * Retry from the first provided password if none of the passwords work In the DeviceConnection class, if none of the passwords appear to work, then try again from the first provided password. The reason for this is if a a device initially needs some password specified in the alternate passwords list, but then later needs to use some earlier-specified password (because of some config change), then connection attempts will fail until a new DeviceConnection object is instantiated. Instead, work around that by trying the passwords in a loop (i.e. from the beginning again). This also means that the class doesn't really need to keep track of what password might be the "primary" password and what password might be alternates. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com> * Remove extra self Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com> --------- Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
5 participants
Description of PR
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
In the DeviceConnection class, if none of the passwords appear to work, then try again from the first provided password. The reason for this is if a a device initially needs some password specified in the alternate passwords list, but then later needs to use some earlier-specified password (because of some config change), then connection attempts will fail until a new DeviceConnection object is instantiated.
How did you do it?
Instead, work around that by trying the passwords in a loop (i.e. from the beginning again). This also means that the class doesn't really need to keep track of what password might be the "primary" password and what password might be alternates.
How did you verify/test it?
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation