Pylint fixes #606
Pylint fixes #606
Conversation
|
Nice! How should we handle this? #607 is more complete, it could make sense that you pick the things you like from this one and we'll close it? |
drivers/VDI.py
Outdated
| @@ -858,13 +858,13 @@ def _create_cbt_log(self): | |||
|
|
|||
| return logpath | |||
|
|
|||
| def _activate_cbt_log(self, logname): | |||
| def _activate_cbt_log(self, logname) -> bool: | |||
There was a problem hiding this comment.
We don't, currently at least, have typing enabled so this is going to have much affect but should be innocuous.
drivers/srmetadata.py
Outdated
| def getVdiInfo(self, Dict, generateSector=0): | ||
| return | ||
| def getVdiInfo(self, Dict, generateSector=0) -> str: | ||
| raise NotImplementedError() |
There was a problem hiding this comment.
This will cause failures, the correct resolution was in #607 so just drop this and the similar change in getSRInfoForSectors
drivers/util.py
Outdated
| try: | ||
| XE_IOFI_IORETRY | ||
| XE_IOFI_IORETRY # pylint: disable=used-before-assignment |
There was a problem hiding this comment.
This whole thing looks truly weird, it was weird before so it's not a reflection of this change but I wonder whether we might not be better seeing if this is actually even used and removing it if not.
There was a problem hiding this comment.
This does indeed seem to be entirely unused and legacy cruft so please just delete the entire if __debug__: block
|
While I was at it I tackled as much as the remaining pylint 2.13.9 errors are possible. A few of them are not so obviously fixed, and I flagged those commits as WIP for discussion. |
drivers/LVHDoISCSISR.py
Outdated
| @@ -492,6 +492,9 @@ def delete(self, sr_uuid): | |||
| def attach(self, sr_uuid): | |||
| try: | |||
| connected = False | |||
| if not self.iscsiSRs: | |||
There was a problem hiding this comment.
instead of if not self.iscsiSRs: you might have better results with if len(self.iscsiSRs) == 0:.
It could also be confused by the try: except: else: construct to set connected=True, that could be moved into the main block and the else: removed.
There was a problem hiding this comment.
Indeed connected mostly duplicates the info of stored_exception, and only using the latter makes the code clearer, helping pylint enough to make sense of it
I think (but will need to dig a bit further |
You mean that's better left for a separate PR ? I guess that would amount to reworking the |
Yeah, I'm doing it now. About to run some tests to make sure nothing explodes in multipath testing, it all seemed to be dangling dead code with no way of triggering the relevant code paths. 207 lines of dead code removed. |
Launching once per module takes much more time, produces more noise, and forces to compute a global result code, for no obvious value. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
A long time ago there was no support for try blocks with both except and finally clauses, but that's the past. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
pylint 2.13.9 does not see that the try block cannot exit before the first line completes: drivers/SMBSR.py:211:37: E0601: Using variable 'err' before assignment (used-before-assignment) Moving the variable out of the try block would not make sense here, so just ignore the error. Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
This is a first quick pass on errors raised by pylint 2.16.4. Nothing critical, mostly cleanups that also improve readability (and as a side effect can help in the future to enable type checking).
The goal is to make #605 mergeable - we're not there yet, but a few similar fixes should do.