Avoid contant disconnect/reconnect in isconnected decorator

[vmatveyevGit]

Current decorator does disconnect and re-connect for every request. That significantly slows down the rate of requests and creates problems with some devices/accounts, that have small number of active sessions allowed, as sessions are not getting removed from the account for some time and sessions limit is getting hit. The proposed decorator sends request first, and if request returns the exception (because of token expiry or session disconnect or any other reason), then decorator will do disconnect and re-connect and will repeat the request. That approach sends all requests over one session and provides automatic recovery when session/token expires. Another part of this change - make consistent exception message in all requests and provide response message/error everywhere, so it is easier to debug those exceptions.

[vmatveyevGit]

Hi, I need help to understand the issue with sanity test. From the error - see below - it looks like "apic" is returning 200 for POST with bogus url and then connection to that apic fails. When I run same post against my real APIC it returns code 404 and re-connection works fine. Can you please check on that and let me know if anything is wrong with my change or it is something happening with test "apic"?

Also - please let me know how to see full console log for the test run - as I only see error, but could not find a link to the full log, to see what exact request and response from "apic".

Thanks, Vladimir

ERROR: test_post_connected_change_expected (test_apic.test_rest_connector)

Traceback (most recent call last): File "/home/runner/work/rest/rest/src/rest/connector/libs/apic/implementation.py", line 183, in decorated ret = func(self, *args, **kwargs) File "/home/runner/work/rest/rest/src/rest/connector/libs/apic/implementation.py", line 349, in post raise RequestException("POST {furl} to {d} has returned the " requests.exceptions.RequestException: POST https://198.51.100.4/temp to apic has returned the following code '200', instead of the expected status code '300', got:

During handling of the above exception, another exception occurred:

Traceback (most recent call last): File "/home/runner/work/rest/rest/src/rest/connector/tests/test_apic.py", line 139, in test_post_connected_changeexpected connection.post(dn='temp', payload={'payload':'something'}, File "src/pyats/async/synchronize.py", line 117, in pyats.async_.synchronize.Lockable.locked.wrapped File "/home/runner/work/rest/rest/src/rest/connector/libs/apic/implementation.py", line 190, in decorated self.connect() File "src/pyats/async/synchronize.py", line 117, in pyats.async_.synchronize.Lockable.locked._wrapped File "/home/runner/work/rest/rest/src/rest/connector/libs/apic/implementation.py", line 155, in connect raise ConnectionError('Connection to {} failed'.format(self.device.name)) ConnectionError: Connection to apic failed

[vmatveyevGit]

Fixed return codes in post testcases for the new decorator.

[omehrabi]

Hi I see you found the issue thanks for contributing to Pyats

[omehrabi]

I think the current logic is not right for example in this scenario if we assume that there is connection and then we try post and it raise an exception the new decorator will catch that but because the self_connected is true we are not creating a new connection thats need to be fixed so I think first we could disconnect in exception and try to connect again the if it failed to connect we will raise an exception but the current implementation just will stuck in recursive calls

[vmatveyevGit]

Was able to run it manually on my build vm and figured out the correct sequence of result codes, that matches new decorator. Passed in my local run.

[vmatveyevGit]

I think first we could disconnect in exception and try to connect again the if it failed to connect we will raise an exception but the current implementation just will stuck in recursive calls

That is exactly what I did in the proposed decorator - if request error/exception happens, then disconnect, then connect again and then re-try the request. If connect fails for any reason, it is not stuck in exception - it throws exception outside with the message that exception happened while already in exception block, and outside function (or outside exception block) gets that exception. So it works as expected and not getting stuck. The test_post_connected_wrong_status actually excersize that condition, by having exception on the second request (after disconnect/reconnect) and that exception is caught and verified by the testcase. The same will happen if connect fails and raises the exception.