Updates due to reviewer comments

salt/utils/files.py

@@ -381,7 +381,8 @@ def fopen(*args, **kwargs):
     # Workaround callers with bad buffering setting for binary files
     if kwargs.get("buffering") == 1 and "b" in kwargs.get("mode", ""):
         log.debug(
-            "Line buffering (buffering=1) isn't supported in binary mode, the default buffer size will be used"
+            "Line buffering (buffering=1) isn't supported in binary mode, "
+            "the default buffer size will be used"
         )
         kwargs["buffering"] = io.DEFAULT_BUFFER_SIZE
 

salt/utils/gitfs.py

@@ -756,7 +756,10 @@ class GitProvider:
         except OSError as exc:
             if exc.errno == errno.ENOENT:
                 # No lock file present
-                msg = f"Attempt to remove lock {self.url} for file ({lock_file}) which does not exist, exception : {exc} "
+                msg = (
+                    f"Attempt to remove lock {self.url} for file ({lock_file}) "
+                    f"which does not exist, exception : {exc} "
+                )
                 log.debug(msg)
 
             elif exc.errno == errno.EISDIR:
@@ -770,7 +773,10 @@ class GitProvider:
             else:
                 _add_error(failed, exc)
         else:
-            msg = f"Removed {lock_type} lock for {self.role} remote '{self.id}' on machine_id '{self.mach_id}'"
+            msg = (
+                f"Removed {lock_type} lock for {self.role} remote '{self.id}' "
+                f"on machine_id '{self.mach_id}'"
+            )
             log.debug(msg)
             success.append(msg)
         return success, failed
@@ -975,8 +981,9 @@ class GitProvider:
                 lock_file = self._get_lock_file(lock_type=lock_type)
                 if self.opts[global_lock_key]:
                     msg = (
-                        f"{global_lock_key} is enabled and {lock_type} lockfile {lock_file} is present for "
+                        f"{global_lock_key} is enabled and {lock_type} lockfile {lock_file} "
-                        f"{self.role} remote '{self.id}' on machine_id {self.mach_id} with pid '{pid}'."
+                        f"is present for {self.role} remote '{self.id}' on machine_id "
+                        f"{self.mach_id} with pid '{pid}'."
                     )
                     if pid:
                         msg += f" Process {pid} obtained the lock"
@@ -1043,7 +1050,10 @@ class GitProvider:
                         raise
                     return
             else:
-                msg = f"Unable to set {lock_type} lock for {self.id} ({self._get_lock_file(lock_type)}) on machine_id {self.mach_id}: {exc}"
+                msg = (
+                    f"Unable to set {lock_type} lock for {self.id} "
+                    f"({self._get_lock_file(lock_type)}) on machine_id {self.mach_id}: {exc}"
+                )
                 log.error(msg, exc_info=True)
                 raise GitLockError(exc.errno, msg)
 

salt/utils/process.py

@@ -1099,8 +1099,9 @@ class SignalHandlingProcess(Process):
                                 os.getpid(),
                             )
 
-                # need to go through and clean up any resources left around like lock files if using gitfs
+                # need to clean up any resources left around like lock files if using gitfs
-                # example lockfile /var/cache/salt/master/gitfs/work/NlJQs6Pss_07AugikCrmqfmqEFrfPbCDBqGLBiCd3oU=/_/update.lk
+                # example: lockfile i
+                # /var/cache/salt/master/gitfs/work/NlJQs6Pss_07AugikCrmqfmqEFrfPbCDBqGLBiCd3oU=/_/update.lk
                 cache_dir = self.opts.get("cachedir", None)
                 gitfs_active = self.opts.get("gitfs_remotes", None)
                 if cache_dir and gitfs_active:
@@ -1131,7 +1132,7 @@ class SignalHandlingProcess(Process):
                                             ).rstrip()
                                         )
                                     except ValueError:
-                                        # Lock file is empty, set mach_id to 0 so it evaluates as False.
+                                        # Lock file is empty, set mach_id to 0 so it evaluates False.
                                         file_mach_id = 0
 
                             if cur_pid == file_pid:
@@ -1163,7 +1164,11 @@ class SignalHandlingProcess(Process):
                             except OSError as exc:
                                 if exc.errno == errno.ENOENT:
                                     # No lock file present
-                                    msg = f"SIGTERM clean up of resources attempted to remove lock file {file_name}, pid '{file_pid}', machine identifier '{mach_id}' but it did not exist, exception : {exc} "
+                                    msg = (
+                                        "SIGTERM clean up of resources attempted to remove lock "
+                                        f"file {file_name}, pid '{file_pid}', machine identifier "
+                                        f"'{mach_id}' but it did not exist, exception : {exc} "
+                                    )
                                     log.debug(msg)
 
                                 elif exc.errno == errno.EISDIR:
@@ -1173,13 +1178,25 @@ class SignalHandlingProcess(Process):
                                     try:
                                         shutil.rmtree(file_name)
                                     except OSError as exc:
-                                        msg = f"SIGTERM clean up of resources, lock file '{file_name}', pid '{file_pid}', machine identifier '{file_mach_id}' was a directory, removed directory, exception : '{exc}'"
+                                        msg = (
+                                            f"SIGTERM clean up of resources, lock file '{file_name}'"
+                                            f", pid '{file_pid}', machine identifier '{file_mach_id}'"
+                                            f"was a directory, removed directory, exception : '{exc}'"
+                                        )
                                         log.debug(msg)
                                 else:
-                                    msg = f"SIGTERM clean up of resources, unable to remove lock file '{file_name}', pid '{file_pid}', machine identifier '{file_mach_id}', exception : '{exc}'"
+                                    msg = (
+                                        "SIGTERM clean up of resources, unable to remove lock file "
+                                        f"'{file_name}', pid '{file_pid}', machine identifier "
+                                        f"'{file_mach_id}', exception : '{exc}'"
+                                    )
                                     log.debug(msg)
                             else:
-                                msg = f"SIGTERM clean up of resources, removed lock file '{file_name}', pid '{file_pid}', machine identifier '{file_mach_id}'"
+                                msg = (
+                                    "SIGTERM clean up of resources, removed lock file "
+                                    f"'{file_name}', pid '{file_pid}', machine identifier "
+                                    f"'{file_mach_id}'"
+                                )
                                 log.debug(msg)
 
             except psutil.NoSuchProcess:

tests/pytests/functional/utils/gitfs/test_gitfs.py

@@ -255,14 +255,20 @@ def _test_lock(opts):
     assert repo.get_salt_working_dir() in repo._get_lock_file()
     assert repo.lock() == (
         [
-            f"Set update lock for gitfs remote 'https://github.com/saltstack/salt-test-pillar-gitfs.git' on machine_id '{mach_id}'"
+            (
+                f"Set update lock for gitfs remote "
+                f"'https://github.com/saltstack/salt-test-pillar-gitfs.git' on machine_id '{mach_id}'"
+            )
         ],
         [],
     )
     assert os.path.isfile(repo._get_lock_file())
     assert repo.clear_lock() == (
         [
-            f"Removed update lock for gitfs remote 'https://github.com/saltstack/salt-test-pillar-gitfs.git' on machine_id '{mach_id}'"
+            (
+                f"Removed update lock for gitfs remote "
+                f"'https://github.com/saltstack/salt-test-pillar-gitfs.git' on machine_id '{mach_id}'"
+            )
         ],
         [],
     )

tests/pytests/unit/utils/test_gitfs_locks.py

@@ -387,7 +387,10 @@ def test_git_provider_mp_gen_lock(main_class, caplog):
     test_msg1 = (
         f"Set update lock for gitfs remote 'file://repo1.git' on machine_id '{mach_id}'"
     )
-    test_msg2 = "Attempting to remove 'update' lock for 'gitfs' remote 'file://repo1.git' due to lock_set1 'True' or lock_set2"
+    test_msg2 = (
+        "Attempting to remove 'update' lock for 'gitfs' remote 'file://repo1.git' "
+        "due to lock_set1 'True' or lock_set2"
+    )
     test_msg3 = f"Removed update lock for gitfs remote 'file://repo1.git' on machine_id '{mach_id}'"
 
     provider = main_class.remotes[0]