Fix the python session to return correct error message when invalid session for MI drivers#2181
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #2181 +/- ##
==========================================
- Coverage 89.85% 89.65% -0.20%
==========================================
Files 73 73
Lines 19006 19048 +42
==========================================
+ Hits 17077 17078 +1
- Misses 1929 1970 +41
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
zoechanzy
force-pushed
the
users/zchan/fix-invalid-session-error-description
branch
from
3c6da41 to
ab14a93
Compare
| error_string = self.error_message(error_code) | ||
| return error_string | ||
| except errors.Error: | ||
| pass |
There was a problem hiding this comment.
It is possible that the session is valid but the returned_error_code unequal to error_code
If this happens, then something likely went wrong retrieving the error message. I'm not sure calling error_message instead of get_error_description is going to change anything in that case.
There was a problem hiding this comment.
Calling get_error_description again might cause infinite loop.
There was a problem hiding this comment.
No, I wasn't suggesting that we call get_error_description again. I'm merely trying to think about what behavior we'll see if this new section of code executes.
Oh, i see. Sorry, I meant get_error, not get_error_description.
There was a problem hiding this comment.
I also find this comment a bit confusing....
What I understand is get_error reads what is currently in the session, if it is overwritten or cleared it could still successfully run but will return mismatched error code.
So this try block is to check the error_message in the current session first before proceed to the set_session_handle in the next try block.
There was a problem hiding this comment.
How about wording the comment to be like this?
# get_error reads the session's error queue, which may have been overwritten,
# causing it to return a mismatched error code. error_message takes the error
# code directly as a parameter and looks up its description without reading the
# queue, so it will just return the description for the specific error code.
# Try it here with the current session before the handle reset in the next block.
|
FYI @zoechanzy , our Linux VM is currently in bad shape, so Linux System tests are going to fail. We hope to have it resolved in the next week or two. |
|
Requested to update description to be more descriptive of the error. Thanks! |
|
Propose to rename the title to be In the PR description Can you explain with screenshot, something like: The expected error message when invalidated NI-SWITCH session, the error message is different between Labview and Python
|
zoechanzy
changed the title
6 participants
What does this Pull Request accomplish?
get_error(reads session error queue)If the session is valid and the error queue has not been overwritten, returned_error_code matches error_code and we return the description immediately.
Reached if Block 1 fell through — either
get_errorraised, or it returned a mismatched code (queue was overwritten). Unlikeget_error,error_messageis a direct lookup byerror_codeand does not read the error queue. If the session is still valid, this returns the correct description without touching the session handle.( second try statement is exactly same with the second try statement in original file. I only updated the docstring section).
Reached only when Block 2 also failed — meaning the error_message in TRY block 2 fail to retrieve error due to internal error or it is an invalid session.
error_messagerejects calls with an invalid session handle, but succeeds withvi=0.List issues fixed by this Pull Request below, if any.
What testing has been done?
Test of invalid session (this bug after fixing):
Unit tests and flake8 passed.