Skip to content

Fix backward compatibility for renamed exception classes (issue #220) #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
guoguojenna wants to merge 6 commits into from
Closed

Fix backward compatibility for renamed exception classes (issue #220) #222

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4dabc7d
Fix backward compatibility for renamed exception classes
guoguojenna Aug 26, 2025
e8dc9ce
Configure ruff to allow assert statements in tests
guoguojenna Aug 26, 2025
663dd42
Delete unit tests
guoguojenna Sep 25, 2025
0196c84
UP031 Use format specifiers instead of percent format
guoguojenna Sep 25, 2025
e34585d
PTH208 Use `pathlib.Path.iterdir()` instead
guoguojenna Sep 25, 2025
b30307b
Fix PTH208 and UP031
guoguojenna Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions rtslib/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,32 +38,36 @@
UserBackedStorageObject,
)
from .utils import (
RTSLibALUANotSupported, # Backward compatibility alias
RTSLibALUANotSupportedError,
RTSLibBrokenLink,
RTSLibError,
RTSLibNotInCFS, # Backward compatibility alias
RTSLibNotInCFSError,
)

__all__ = [
"RTSRoot",
"RTSLibError",
"RTSLibBrokenLink",
"RTSLibNotInCFSError",
"RTSLibALUANotSupportedError",
"LUN",
"MappedLUN",
"NodeACL",
"NetworkPortal",
"TPG",
"Target",
"NodeACLGroup",
"MappedLUNGroup",
"ALUATargetPortGroup",
"BlockStorageObject",
"FabricModule",
"FileIOStorageObject",
"BlockStorageObject",
"MappedLUN",
"MappedLUNGroup",
"NetworkPortal",
"NodeACL",
"NodeACLGroup",
"PSCSIStorageObject",
"RDMCPStorageObject",
"UserBackedStorageObject",
"RTSLibALUANotSupported", # Backward compatibility alias
"RTSLibALUANotSupportedError",
"RTSLibBrokenLink",
"RTSLibError",
"RTSLibNotInCFS", # Backward compatibility alias
"RTSLibNotInCFSError",
"RTSRoot",
"StorageObjectFactory",
"ALUATargetPortGroup",
"Target",
"UserBackedStorageObject",
]
2 changes: 1 addition & 1 deletion rtslib/alua.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def __init__(self, storage_object, name, tag=None):
fwrite(f"{self._path}/tg_pt_gp_id", tag)
except OSError as msg:
self.delete()
raise RTSLibError("Cannot set id to %d: %s" % (tag, str(msg)))
raise RTSLibError(f"Cannot set id to {tag}: {msg!s}")
else:
try:
self._create_in_cfs_ine('lookup')
Expand Down
2 changes: 1 addition & 1 deletion rtslib/fabric.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,6 @@ def all(cls):
@classmethod
def list_registered_drivers(cls):
try:
return os.listdir('/sys/module/target_core_mod/holders')
return [p.name for p in Path('/sys/module/target_core_mod/holders').iterdir()]
except OSError:
return []
6 changes: 3 additions & 3 deletions rtslib/root.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ def err_func(err_str):

for index, so in enumerate(config.get('storage_objects', [])):
if 'name' not in so:
err_func("'name' not defined in storage object %d" % index)
err_func(f"'name' not defined in storage object {index}")
continue

# * Restore/load the single matching storage object if
Expand Down Expand Up @@ -400,7 +400,7 @@ def so_err_func(x):
# Don't need to create fabric modules
for index, fm in enumerate(config.get('fabric_modules', [])):
if 'name' not in fm:
err_func("'name' not defined in fabricmodule %d" % index)
err_func(f"'name' not defined in fabricmodule {index}")
continue
for fm_obj in self.fabric_modules:
if fm['name'] == fm_obj.name:
Expand All @@ -409,7 +409,7 @@ def so_err_func(x):

for index, t in enumerate(config.get('targets', [])):
if 'wwn' not in t:
err_func("'wwn' not defined in target %d" % index)
err_func(f"'wwn' not defined in target {index}")
continue

# * Restore/load the single matching target if target=iqn.xxx was
Expand Down
44 changes: 20 additions & 24 deletions rtslib/target.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@
# TPG private stuff

def __repr__(self):
return "<TPG %d>" % self.tag
return f"<TPG {self.tag}>"

def __init__(self, parent_target, tag=None, mode='any'):
'''
Expand Down Expand Up @@ -197,7 +197,7 @@
else:
raise RTSLibError("Invalid parent Target")

self._path = "%s/tpgt_%d" % (self.parent_target.path, self.tag)
self._path = f"{self.parent_target.path}/tpgt_{self.tag}"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class Warning

Assignment overwrites attribute _path, which was previously defined in superclass
CFSNode
Loading
.

Copilot Autofix

AI 3 months ago

The recommended approach is to avoid direct attribute overwriting. In this case:

  1. Update the CFSNode superclass (if possible—given only what we see here) so that _path is either passed as a parameter or defined only in the superclass from a property/setter.
  2. Since we cannot modify the superclass (code not shown), and this method is in a subclass, the best fix is to:
    • Instead of setting self._path directly in the subclass, pass the full path as a parameter to the superclass constructor via super().__init__(), if supported and if the constructor signature allows.
    • If the superclass does not accept a path as a constructor argument, then define a new uniquely-named attribute in the subclass (e.g., self._tpg_path), and use that in this subclass instead of self._path.
    • Refactor all uses of self._path in this subclass to self._tpg_path.

Specifically:

  • In the code block, replace assignment to self._path = ... with self._tpg_path = ....
  • Update subsequent code in this method (and elsewhere in this class, if needed) that references self._path to self._tpg_path.
  • Do not assign directly to self._path in the subclass.
  • No additional imports or new methods are needed.
Suggested changeset 1
rtslib/target.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/rtslib/target.py b/rtslib/target.py
--- a/rtslib/target.py
+++ b/rtslib/target.py
@@ -197,9 +197,9 @@
         else:
             raise RTSLibError("Invalid parent Target")
 
-        self._path = f"{self.parent_target.path}/tpgt_{self.tag}"
+        self._tpg_path = f"{self.parent_target.path}/tpgt_{self.tag}"
 
-        path = Path(self._path)
+        path = Path(self._tpg_path)
         target_path = Path(self.parent_target.path)
         if not self.has_feature('tpgts') and not path.is_dir():
             for filename in target_path.iterdir():
EOF
@@ -197,9 +197,9 @@
else:
raise RTSLibError("Invalid parent Target")

self._path = f"{self.parent_target.path}/tpgt_{self.tag}"
self._tpg_path = f"{self.parent_target.path}/tpgt_{self.tag}"

path = Path(self._path)
path = Path(self._tpg_path)
target_path = Path(self.parent_target.path)
if not self.has_feature('tpgts') and not path.is_dir():
for filename in target_path.iterdir():
Copilot is powered by AI and may make mistakes. Always verify output.

path = Path(self._path)
target_path = Path(self.parent_target.path)
Expand Down Expand Up @@ -482,8 +482,7 @@
# LUN private stuff

def __repr__(self):
return "<LUN %d (%s/%s)>" % (self.lun, self.storage_object.plugin,
self.storage_object.name)
return f"<LUN {self.lun} ({self.storage_object.plugin}/{self.storage_object.name})>"

def __init__(self, parent_tpg, lun=None, storage_object=None, alias=None):
'''
Expand Down Expand Up @@ -522,15 +521,15 @@
lun = index
break
if lun is None:
raise RTSLibError("All LUNs 0-%d in use" % self.MAX_TARGET_LUN)
raise RTSLibError(f"All LUNs 0-{self.MAX_TARGET_LUN} in use")
else:
lun = int(lun)
if lun < 0 or lun > self.MAX_TARGET_LUN:
raise RTSLibError("LUN must be 0 to %d" % self.MAX_TARGET_LUN)
raise RTSLibError(f"LUN must be 0 to {self.MAX_TARGET_LUN}")

self._lun = lun

self._path = "%s/lun/lun_%d" % (self.parent_tpg.path, self.lun)
self._path = f"{self.parent_tpg.path}/lun/lun_{self.lun}"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class Warning

Assignment overwrites attribute _path, which was previously defined in superclass
CFSNode
Loading
.

Copilot Autofix

AI 3 months ago

To fix the problem, we need to avoid directly overwriting the _path attribute in the subclass after it has been set by the superclass. The best approach is to ensure that the intended value for _path is set centrally during initialization, ideally through a parameter passed to the superclass's __init__ method if possible.

Assuming that CFSNode currently sets self._path in its __init__, the code can be refactored as follows:

  • Alter CFSNode.__init__ to accept a path parameter (defaulting to None if necessary) and set self._path = path if provided.
  • In the LUN's __init__, compute the intended path string before calling super().__init__(), and pass it as a parameter, so the superclass sets _path, and no overwrite is needed.

All changes should be limited to the LUN class definition within rtslib/target.py, since that's the code provided.

If the superclass already accepts parameters for custom path initialization, use that parameter as intended. Otherwise, refactor as above, within the visibility of the provided code. If that's not possible, then the subclass should use a different attribute name (e.g., _lun_path) to avoid shadowing the superclass's _path.

Suggested changeset 1
rtslib/target.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/rtslib/target.py b/rtslib/target.py
--- a/rtslib/target.py
+++ b/rtslib/target.py
@@ -507,8 +507,6 @@
         @type alias: string
         @return: A LUN object.
         '''
-        super().__init__()
-
         if isinstance(parent_tpg, TPG):
             self._parent_tpg = parent_tpg
         else:
@@ -528,9 +526,9 @@
                 raise RTSLibError(f"LUN must be 0 to {self.MAX_TARGET_LUN}")
 
         self._lun = lun
+        lun_path = f"{self.parent_tpg.path}/lun/lun_{self.lun}"
+        super().__init__(path=lun_path)
 
-        self._path = f"{self.parent_tpg.path}/lun/lun_{self.lun}"
-
         if storage_object is None and alias is not None:
             raise RTSLibError("The alias parameter has no meaning "
                               "without the storage_object parameter")
EOF
@@ -507,8 +507,6 @@
@type alias: string
@return: A LUN object.
'''
super().__init__()

if isinstance(parent_tpg, TPG):
self._parent_tpg = parent_tpg
else:
@@ -528,9 +526,9 @@
raise RTSLibError(f"LUN must be 0 to {self.MAX_TARGET_LUN}")

self._lun = lun
lun_path = f"{self.parent_tpg.path}/lun/lun_{self.lun}"
super().__init__(path=lun_path)

self._path = f"{self.parent_tpg.path}/lun/lun_{self.lun}"

if storage_object is None and alias is not None:
raise RTSLibError("The alias parameter has no meaning "
"without the storage_object parameter")
Copilot is powered by AI and may make mistakes. Always verify output.

if storage_object is None and alias is not None:
raise RTSLibError("The alias parameter has no meaning "
Expand Down Expand Up @@ -766,28 +765,27 @@
@classmethod
def setup(cls, tpg_obj, lun, err_func):
if 'index' not in lun:
err_func("'index' missing from a LUN in TPG %d" % tpg_obj.tag)
err_func(f"'index' missing from a LUN in TPG {tpg_obj.tag}")
return

try:
bs_name, so_name = lun['storage_object'].split('/')[2:]
except:
err_func("Malformed storage object field for LUN %d" % lun['index'])
err_func(f"Malformed storage object field for LUN {lun['index']}")
return

for so in tcm.StorageObject.all():
if so_name == so.name and bs_name == so.plugin:
match_so = so
break
else:
err_func("Could not find matching StorageObject for LUN %d" % lun['index'])
err_func(f"Could not find matching StorageObject for LUN {lun['index']}")
return

try:
lun_obj = cls(tpg_obj, lun['index'], storage_object=match_so, alias=lun.get('alias'))
except (RTSLibError, KeyError):
err_func("Creating TPG %d LUN index %d failed" %
(tpg_obj.tag, lun['index']))
err_func(f"Creating TPG {tpg_obj.tag} LUN index {lun['index']} failed")

# alua_tg_pt_gp support not present in older versions
with contextlib.suppress(KeyError):
Expand Down Expand Up @@ -847,8 +845,7 @@
else:
raise RTSLibError("Invalid parent TPG")

self._path = "%s/np/%s:%d" \
% (self.parent_tpg.path, self.ip_address, self.port)
self._path = f"{self.parent_tpg.path}/np/{self.ip_address}:{self.port}"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class Warning

Assignment overwrites attribute _path, which was previously defined in superclass
CFSNode
Loading
.

Copilot Autofix

AI 3 months ago

The best way to fix this problem is to avoid reassigning the _path attribute directly in the subclass if it is already set in the superclass. Instead, _path should be managed centrally in the superclass, ideally via an __init__ argument if the value needs to be set in a subclass-specific way, or through a property if dynamic calculation is needed.

General Approach:

  • Refactor the superclass (CFSNode) to accept _path as an argument to its __init__ method.
  • In the subclass's __init__, calculate the appropriate path string first, then pass it to the superclass via super().__init__(_path=calculated_path, ...).
  • Remove all direct assignments to self._path in the subclass.

What/where to change:

  • In the code for NetworkPortal (or the specific subclass), replace the direct assignment of self._path = ... with a calculation before super().__init__, and pass the result into the superclass's constructor.
  • Update the call to super().__init__() on line 834 to pass in the calculated _path.

NOTE: We are only allowed to update code seen in the snippet. As such, the fix will focus on moving self._path computation before super() and passing it as an argument, and assume that CFSNode can accept this (otherwise we'd need to update CFSNode, which is out of scope here).

Suggested changeset 1
rtslib/target.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/rtslib/target.py b/rtslib/target.py
--- a/rtslib/target.py
+++ b/rtslib/target.py
@@ -831,8 +831,6 @@
         @type mode:string
         @return: A NetworkPortal object.
         '''
-        super().__init__()
-
         self._ip_address = str(ip_address)
 
         try:
@@ -845,8 +843,10 @@
         else:
             raise RTSLibError("Invalid parent TPG")
 
-        self._path = f"{self.parent_tpg.path}/np/{self.ip_address}:{self.port}"
+        _path = f"{self.parent_tpg.path}/np/{self.ip_address}:{self.port}"
 
+        super().__init__(_path=_path)
+
         try:
             self._create_in_cfs_ine(mode)
         except OSError as msg:
EOF
@@ -831,8 +831,6 @@
@type mode:string
@return: A NetworkPortal object.
'''
super().__init__()

self._ip_address = str(ip_address)

try:
@@ -845,8 +843,10 @@
else:
raise RTSLibError("Invalid parent TPG")

self._path = f"{self.parent_tpg.path}/np/{self.ip_address}:{self.port}"
_path = f"{self.parent_tpg.path}/np/{self.ip_address}:{self.port}"

super().__init__(_path=_path)

try:
self._create_in_cfs_ine(mode)
except OSError as msg:
Copilot is powered by AI and may make mistakes. Always verify output.

try:
self._create_in_cfs_ine(mode)
Expand Down Expand Up @@ -918,10 +915,10 @@
@classmethod
def setup(cls, tpg_obj, p, err_func):
if 'ip_address' not in p:
err_func("'ip_address' field missing from a portal in TPG %d" % tpg_obj.tag)
err_func(f"'ip_address' field missing from a portal in TPG {tpg_obj.tag}")
return
if 'port' not in p:
err_func("'port' field missing from a portal in TPG %d" % tpg_obj.tag)
err_func(f"'port' field missing from a portal in TPG {tpg_obj.tag}")
return

try:
Expand Down Expand Up @@ -1165,9 +1162,8 @@
# MappedLUN private stuff

def __repr__(self):
return "<MappedLUN %s lun %d -> tpg%d lun %d>" % \
(self.parent_nodeacl.node_wwn, self.mapped_lun,
self.parent_nodeacl.parent_tpg.tag, self.tpg_lun.lun)
return (f"<MappedLUN {self.parent_nodeacl.node_wwn} lun {self.mapped_lun} -> "
f"tpg{self.parent_nodeacl.parent_tpg.tag} lun {self.tpg_lun.lun}>")

def __init__(self, parent_nodeacl, mapped_lun,
tpg_lun=None, write_protect=None, alias=None):
Expand Down Expand Up @@ -1209,7 +1205,7 @@
if self._mapped_lun < 0:
raise RTSLibError("Mapped LUN must be >= 0")

self._path = "%s/lun_%d" % (self.parent_nodeacl.path, self.mapped_lun)
self._path = f"{self.parent_nodeacl.path}/lun_{self.mapped_lun}"

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class Warning

Assignment overwrites attribute _path, which was previously defined in superclass
CFSNode
Loading
.

Copilot Autofix

AI 3 months ago

To fix the problem, ensure that the _path attribute is not directly overwritten in the subclass after being set in the superclass. Instead, pass the correct _path value up to the superclass __init__ method, delegating initialization properly.

  • Determine where CFSNode.__init__() sets _path and refactor its constructor to receive _path as an optional argument, defaulting to its previous logic if not provided.
  • In the subclass's __init__, compute the intended _path before calling super().__init__, and pass it as an argument to the superclass constructor.
  • Remove the direct subclass assignment to self._path; all initialization should go through the superclass.

You only need to modify the area in rtslib/target.py that defines the subclass and the construction of _path in its __init__ method. If the superclass is not shown here, you can only edit what is visible in the subclass.

Suggested changeset 1
rtslib/target.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/rtslib/target.py b/rtslib/target.py
--- a/rtslib/target.py
+++ b/rtslib/target.py
@@ -1189,7 +1189,8 @@
         @type write_protect: bool
         '''
 
-        super().__init__()
+        path = f"{parent_nodeacl.path}/lun_{int(mapped_lun)}"
+        super().__init__(_path=path)
 
         if not isinstance(parent_nodeacl, NodeACL):
             raise RTSLibError("The parent_nodeacl parameter must be a NodeACL object")
@@ -1205,7 +1206,7 @@
         if self._mapped_lun < 0:
             raise RTSLibError("Mapped LUN must be >= 0")
 
-        self._path = f"{self.parent_nodeacl.path}/lun_{self.mapped_lun}"
+        # self._path assignment moved into superclass constructor
 
         if tpg_lun is None and write_protect is not None:
             raise RTSLibError("The write_protect parameter has no "
EOF
@@ -1189,7 +1189,8 @@
@type write_protect: bool
'''

super().__init__()
path = f"{parent_nodeacl.path}/lun_{int(mapped_lun)}"
super().__init__(_path=path)

if not isinstance(parent_nodeacl, NodeACL):
raise RTSLibError("The parent_nodeacl parameter must be a NodeACL object")
@@ -1205,7 +1206,7 @@
if self._mapped_lun < 0:
raise RTSLibError("Mapped LUN must be >= 0")

self._path = f"{self.parent_nodeacl.path}/lun_{self.mapped_lun}"
# self._path assignment moved into superclass constructor

if tpg_lun is None and write_protect is not None:
raise RTSLibError("The write_protect parameter has no "
Copilot is powered by AI and may make mistakes. Always verify output.

if tpg_lun is None and write_protect is not None:
raise RTSLibError("The write_protect parameter has no "
Expand Down Expand Up @@ -1336,8 +1332,8 @@
tpg_lun_obj = lun
break
else:
err_func("Could not find matching TPG LUN %d for MappedLUN %s" %
(mlun['tpg_lun'], mlun['index']))
err_func(f"Could not find matching TPG LUN {mlun['tpg_lun']} "
f"for MappedLUN {mlun['index']}")
return

try:
Expand All @@ -1346,7 +1342,7 @@
mlun.get('alias'))
mlun_obj.tag = mlun.get("tag", None)
except (RTSLibError, KeyError):
err_func("Creating MappedLUN object %d failed" % mlun['index'])
err_func(f"Creating MappedLUN object {mlun['index']} failed")

def dump(self):
d = super().dump()
Expand Down Expand Up @@ -1640,7 +1636,7 @@
'''

def __repr__(self):
return "<MappedLUNGroup %s:lun %d>" % (self._nag.name, self._mapped_lun)
return f"<MappedLUNGroup {self._nag.name}:lun {self._mapped_lun}>"

def __init__(self, nodeaclgroup, mapped_lun, *args, **kwargs):
super().__init__(MappedLUNGroup._mapped_luns.fget)
Expand Down
7 changes: 7 additions & 0 deletions rtslib/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ class RTSLibNotInCFSError(RTSLibError):
object that does not exist.
'''


# Backward compatibility aliases for the old exception names
# These were renamed in commit fdd69b1 to follow PEP8 naming conventions
# but broke backward compatibility. Keep the old names as aliases.
RTSLibALUANotSupported = RTSLibALUANotSupportedError
RTSLibNotInCFS = RTSLibNotInCFSError

def fwrite(path, string):
'''
This function writes a string to a file, and takes care of
Expand Down