From fa49159de52928944749cb8bafb50010e6654e79 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Mon, 15 Mar 2021 11:20:46 +0000 Subject: [PATCH 1/2] JournalLock: Catch when journal_dir is None * If it's None then set journal_dir_path to None as well. Setting '' or nothing results in '.' (CWD), which could cause other issues. * As we do this in three places, it's in a helper function. * New JournalLockResult.JOURNALDIR_IS_NONE to signal this. * Fix checking of return from obtain_lock() to specifically reference JournalLockResult.ALREADY_LOCKED. --- journal_lock.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/journal_lock.py b/journal_lock.py index e947ffe1..068626a4 100644 --- a/journal_lock.py +++ b/journal_lock.py @@ -25,6 +25,7 @@ class JournalLockResult(Enum): JOURNALDIR_NOTEXIST = 2 JOURNALDIR_READONLY = 3 ALREADY_LOCKED = 4 + JOURNALDIR_IS_NONE = 5 class JournalLock: @@ -33,18 +34,33 @@ class JournalLock: def __init__(self) -> None: """Initialise where the journal directory and lock file are.""" self.journal_dir: str = config.get('journaldir') or config.default_journal_dir - self.journal_dir_path = pathlib.Path(self.journal_dir) + self.journal_dir_path: Optional[pathlib.Path] = None + self.set_path_from_journaldir() self.journal_dir_lockfile_name: Optional[pathlib.Path] = None # We never test truthiness of this, so let it be defined when first assigned. Avoids type hint issues. # self.journal_dir_lockfile: Optional[IO] = None self.locked = False + def set_path_from_journaldir(self): + if self.journal_dir is None: + self.journal_dir_path = None + + else: + try: + self.journal_dir_path = pathlib.Path(self.journal_dir) + + except Exception: + logger.exception("Couldn't make pathlib.Path from journal_dir", exc_info=True) + def obtain_lock(self) -> JournalLockResult: """ Attempt to obtain a lock on the journal directory. :return: LockResult - See the class Enum definition """ + if self.journal_dir_path is None: + return JournalLockResult.JOURNALDIR_IS_NONE + self.journal_dir_lockfile_name = self.journal_dir_path / 'edmc-journal-lock.txt' logger.trace(f'journal_dir_lockfile_name = {self.journal_dir_lockfile_name!r}') try: @@ -100,7 +116,7 @@ class JournalLock: """ Release lock on journal directory. - :return: bool - Success of unlocking operation. + :return: bool - Whether we're now unlocked. """ if not self.locked: return True # We weren't locked, and still aren't @@ -222,8 +238,9 @@ class JournalLock: self.release_lock() self.journal_dir = current_journaldir - self.journal_dir_path = pathlib.Path(self.journal_dir) - if not self.obtain_lock(): + self.set_path_from_journaldir() + + if self.obtain_lock() == JournalLockResult.ALREADY_LOCKED: # Pop-up message asking for Retry or Ignore self.retry_popup = self.JournalAlreadyLocked(parent, self.retry_lock) @@ -241,7 +258,7 @@ class JournalLock: current_journaldir = config.get('journaldir') or config.default_journal_dir self.journal_dir = current_journaldir - self.journal_dir_path = pathlib.Path(self.journal_dir) - if not self.obtain_lock(): + self.set_path_from_journaldir() + if self.obtain_lock() == JournalLockResult.ALREADY_LOCKED: # Pop-up message asking for Retry or Ignore self.retry_popup = self.JournalAlreadyLocked(parent, self.retry_lock) From f1c50faafa1f503bc4882f205368ba41aef7d346 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Mon, 15 Mar 2021 15:58:18 +0000 Subject: [PATCH 2/2] logger.exception() doesn't need exc_info --- journal_lock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/journal_lock.py b/journal_lock.py index 068626a4..51f94f89 100644 --- a/journal_lock.py +++ b/journal_lock.py @@ -50,7 +50,7 @@ class JournalLock: self.journal_dir_path = pathlib.Path(self.journal_dir) except Exception: - logger.exception("Couldn't make pathlib.Path from journal_dir", exc_info=True) + logger.exception("Couldn't make pathlib.Path from journal_dir") def obtain_lock(self) -> JournalLockResult: """