Use subprocess.run(); lots of lint updates

Signed-off-by: Dean Roehrich <[email protected]>

tools/crd-bumper/pkg/controllers.py

@@ -23,6 +23,7 @@
 
 
 class Controllers:
+    """Point controllers and spokes at new hub."""
 
     def __init__(
         self,
@@ -35,7 +36,7 @@ def __init__(
         topdir=None,
     ):
         if not isinstance(project, Project):
-            raise Exception("need a Project")
+            raise TypeError("need a Project")
         self._dryrun = dryrun
         self._project = project
         self._prev_ver = prev_ver
@@ -45,13 +46,15 @@ def __init__(
         self._topdir = topdir
 
     def has_earlier_spokes(self):
+        """Determine whether the repo has enough API versions to have an existing spoke."""
 
         for _, dir_names, _ in os.walk("api", followlinks=False):
             if len(dir_names) > 2:
                 return True
         return False
 
     def bump_earlier_spokes(self):
+        """If the repo has earlier spokes, update them to point at the new hub."""
 
         earlier_spokes = self.has_earlier_spokes()
         if earlier_spokes:
@@ -65,14 +68,15 @@ def edit_util_conversion_test(self):
 
         kinds = self._project.kinds(self._prev_ver)
         if len(kinds) == 0:
-            raise Exception(f"Nothing found at version {self._prev_ver}")
+            raise ValueError(f"Nothing found at version {self._prev_ver}")
         self._walk_files("github/cluster-api/util/conversion", kinds)
 
     def run(self):
+        """Walk over APIs and controllers, bumping them to point at the new hub."""
 
         kinds = self._project.kinds(self._prev_ver)
         if len(kinds) == 0:
-            raise Exception(f"Nothing found at version {self._prev_ver}")
+            raise ValueError(f"Nothing found at version {self._prev_ver}")
 
         if self._topdir is not None:
             self._walk_files(self._topdir, kinds)
@@ -82,6 +86,8 @@ def run(self):
                     self._walk_files(top, kinds)
 
     def _walk_files(self, top, kinds):
+        """Walk the files in the given directory, and update them to point at the new hub."""
+
         for root, _, f_names in os.walk(top, followlinks=False):
             for fname in f_names:
                 this_api = os.path.basename(root)
@@ -107,6 +113,7 @@ def _walk_files(self, top, kinds):
                     self._point_at_new_hub(kinds, full_path)
 
     def _point_at_new_hub(self, kinds, path):
+        """Update the given file to point it at the new hub."""
 
         fu = FileUtil(self._dryrun, path)
         for k in kinds:
@@ -131,6 +138,7 @@ def _point_at_new_hub(self, kinds, path):
             fu.store()
 
     def _update_suite_test(self, kinds, path):
+        """Update the suite_test.go file to include the setup of the new hub."""
 
         fu = FileUtil(self._dryrun, path)
 
@@ -184,6 +192,7 @@ def _conversiongen_marker(self, path, this_api):
             fu.store()
 
     def _update_spoke_conversion(self, kinds, path, this_api):
+        """Update the conversion.go in each pre-existing spoke to point at the new hub."""
 
         if this_api == self._prev_ver:
             # Adjust only the established spokes; the new spoke is already correct.

tools/crd-bumper/pkg/conversion_gen.py

@@ -24,10 +24,11 @@
 
 
 class ConversionGen:
+    """Create conversion routines and tests."""
 
     def __init__(self, dryrun, project, prev_ver, new_ver, most_recent_spoke):
         if not isinstance(project, Project):
-            raise Exception("need a Project")
+            raise TypeError("need a Project")
         self._dryrun = dryrun
         self._project = project
         self._prev_ver = prev_ver
@@ -109,21 +110,23 @@ def fix_kubebuilder_import_alias(self):
         #    fu.store()
 
     def module(self):
+        """Return the name of this Go module."""
+
         if self._module is not None:
             return self._module
 
         go_mod = "go.mod"
         if not os.path.isfile(go_mod):
-            raise Exception(f"unable to find {go_mod}")
+            raise FileNotFoundError(f"unable to find {go_mod}")
 
         # Get the module name.
         fu = FileUtil(self._dryrun, go_mod)
         module_line = fu.find_with_pattern("^module ")
         if module_line is None:
-            raise Exception(f"unable to find module name in {go_mod}")
+            raise ValueError(f"unable to find module name in {go_mod}")
         m = re.search(r"^module\s+(.*)", module_line)
         if m is None:
-            raise Exception(f"unable to parse module name in {go_mod}")
+            raise ValueError(f"unable to parse module name in {go_mod}")
         self._module = m.group(1)
         return self._module
 
@@ -152,7 +155,7 @@ def _add_import_for_new_hub(self, kinds, fu):
         Update the file to add an import for the new hub.
         """
         if not isinstance(fu, FileUtil):
-            raise Exception("need a FileUtil")
+            raise TypeError("need a FileUtil")
 
         if self._preferred_alias is None:
             # Pick the first kind, use its group.
@@ -302,7 +305,7 @@ def mk_spoke(self):
 
     def _update_spoke_conversion(self, kinds, fu):
         if not isinstance(fu, FileUtil):
-            raise Exception("need a FileUtil")
+            raise TypeError("need a FileUtil")
 
         if self._preferred_alias is None:
             # Pick the first kind, use its group.

tools/crd-bumper/pkg/copyright.py

@@ -22,12 +22,15 @@
 
 
 class Copyright:
+    """Update copyright on files."""
 
     def __init__(self, dryrun):
         self._dryrun = dryrun
         self._year = str(datetime.date.today().year)
 
     def update(self, path):
+        """Search for copyright strings and update the dates."""
+
         fu = FileUtil(self._dryrun, path)
         line = fu.find_in_file("Copyright")
         if line is None:

tools/crd-bumper/pkg/create_apis.py

@@ -25,10 +25,11 @@
 
 
 class CreateApis:
+    """Create the new API versions."""
 
     def __init__(self, dryrun, project, prev_ver, new_ver, preferred_alias, module):
         if not isinstance(project, Project):
-            raise Exception("need a Project")
+            raise TypeError("need a Project")
         self._dryrun = dryrun
         self._project = project
         self._prev_ver = prev_ver
@@ -55,10 +56,11 @@ def prev_is_hub(self):
         return True
 
     def create(self):
+        """For each kind, create a new API version using kubebuilder."""
 
         kinds = self._project.kinds(self._prev_ver)
         if len(kinds) == 0:
-            raise Exception(f"Nothing found at version {self._prev_ver}")
+            raise ValueError(f"Nothing found at version {self._prev_ver}")
         kinds_new_ver = self._project.kinds(self._new_ver)
 
         for k in kinds:
@@ -70,27 +72,26 @@ def create(self):
                     print(f"Dryrun: {cmd}")
                 else:
                     print(f"Running: {cmd}")
-                    child = subprocess.Popen(
+                    res = subprocess.run(
                         shlex.split(cmd),
-                        shell=False,
-                        stdout=subprocess.PIPE,
-                        stderr=subprocess.PIPE,
+                        capture_output=True,
                         text=True,
+                        check=False,
                     )
-                    res = child.communicate()
-                    if child.returncode != 0:
-                        raise Exception(
-                            f"Unable to create API for {k}.{self._new_ver}:\n{res[1]}"
+                    if res.returncode != 0:
+                        raise RuntimeError(
+                            f"Unable to create API for {k}.{self._new_ver}:\n{res.stderr}"
                         )
 
     def copy_content(self, git):
+        """Copy the API content from the previous hub to the new hub."""
 
         kinds_prev_ver = self._project.kinds(self._prev_ver)
         if len(kinds_prev_ver) == 0:
-            raise Exception(f"Nothing found at version {self._prev_ver}")
+            raise ValueError(f"Nothing found at version {self._prev_ver}")
         kinds_new_ver = self._project.kinds(self._new_ver)
 
-        copied = dict()
+        copied = {}
         for k in kinds_new_ver:
             if k in kinds_prev_ver:
                 src = f"api/{self._prev_ver}/{k.lower()}_types.go"
@@ -159,7 +160,7 @@ def set_storage_version(self):
                 if line is None:
                     line = fu.find_in_file("+kubebuilder:object:root=true")
                 if line is None:
-                    raise Exception(
+                    raise LookupError(
                         f"Unable to place kubebuilder:storageversion in {fname}"
                     )
                 fu.replace_in_file(line, f"""{line}\n// +kubebuilder:storageversion""")
@@ -182,6 +183,10 @@ def add_conversion_schemebuilder(self):
         fu.store()
 
     def edit_new_api_files(self):
+        """
+        Update the API version reference in the Go files that have content that
+        was relocated from the previous hub to the new hub.
+        """
 
         kinds = self._project.kinds(self._new_ver)
         for root, _, f_names in os.walk(f"api/{self._new_ver}", followlinks=False):
@@ -205,7 +210,7 @@ def edit_new_api_files(self):
                             # Before: '\tdwsv1alpha1 "github.com/hewpack/dws/api/v1alpha1"'
                             # After:  '\tdwsv1alpha2 "github.com/hewpack/dws/api/v1alpha2"'
                             line2 = f'\t{group}{self._new_ver} "{self._module}/api/{self._new_ver}"'
-                            fu.replace_in_file(line, new)
+                            fu.replace_in_file(line, line2)
 
                         # This matches: dwsv1alpha1.  (yes, dot)
                         fu.replace_in_file(

tools/crd-bumper/pkg/fileutil.py

@@ -20,13 +20,16 @@
 
 
 class FileUtil:
+    """Read and update files."""
 
     def __init__(self, dryrun, fpath):
         self._dryrun = dryrun
         self._fpath = fpath
         self._input_data = None
 
     def read(self):
+        """Read the file if it has not already been read."""
+
         if self._input_data is None:
             try:
                 with open(self._fpath, "r", encoding="utf-8") as f1:
@@ -36,27 +39,30 @@ def read(self):
                 raise
 
     def store(self):
+        """Store the file if it has data to be stored."""
+
         if self._input_data is not None:
-            f2 = open(f"{self._fpath}.new", "w", encoding="utf-8")
-            f2.write(self._input_data)
-            f2.close()
+            with open(f"{self._fpath}.new", "w", encoding="utf-8") as f2:
+                f2.write(self._input_data)
 
             if not self._dryrun:
                 os.rename(f"{self._fpath}.new", self._fpath)
 
     def replace_in_file(self, from_str, to_str):
+        """Replace one string with another throughout the file."""
+
         self.read()
         changed = False
         if self._input_data is not None:
             input_data = self._input_data.replace(from_str, to_str)
             if input_data != self._input_data:
                 changed = True
                 self._input_data = input_data
-                if self._input_data is None:
-                    raise Exception("Hey, we lost the input data")
         return changed
 
     def delete_from_file(self, from_str):
+        """Delete all instances of a line from the file."""
+
         self.read()
         changed = False
         if self._input_data is not None:
@@ -70,6 +76,8 @@ def delete_from_file(self, from_str):
         return changed
 
     def find_in_file(self, substr):
+        """Find the first line having a given substring."""
+
         self.read()
         if self._input_data is not None:
             for line in self._input_data.split("\n"):
@@ -78,6 +86,8 @@ def find_in_file(self, substr):
         return None
 
     def find_with_pattern(self, pat):
+        """Find the first line matching a given pattern."""
+
         self.read()
         if self._input_data is not None:
             for line in self._input_data.split("\n"):
@@ -86,6 +96,8 @@ def find_with_pattern(self, pat):
         return None
 
     def append(self, line):
+        """Append the given line to the file."""
+
         self.read()
         if self._input_data is None:
             self._input_data = ""