Skip to content

Pre-existing code quality findings from Ruff PR review #289

@nitrobass24

Description

@nitrobass24

Summary

During the Ruff linting PR (#288) review, several pre-existing code quality issues were identified. None were introduced by the Ruff changes.

Findings to address

Real bugs

  • Busy-wait CPU spike on shutdown (common/app_process.py:114-115) — while self.is_alive(): pass burns CPU. Fix: add self.join(timeout=0.1) or time.sleep(0.05) inside the loop.

  • Race condition on queue internals (controller/extract/dispatch.py:104-112) — list(self.__task_queue.queue) reads the raw deque without holding the queue mutex. Fix: wrap with with self.__task_queue.mutex:.

  • user=None produces "None@host" (ssh/sshcp.py:31-38) — Constructor accepts user=None but callers format "{user}@{host}" unconditionally. Fix: add a _remote_target() helper that omits user when None.

  • Wrong boolean in wait loop (tests/unittests/test_controller/test_extract/test_dispatch.py:704-705) — Uses and instead of or, causing premature exit. Fix: change to while ... < 1 or ... < 1.

Minor / resource leaks

  • urlopen not closed (controller/notifier.py:114-115) — Response not in context manager on success path. Minor socket leak.

  • Popen not awaited, fnull not closed (tests/integration/test_controller/test_extract/test_extract.py:49-64) — Archive creation subprocesses may not finish before tests run. Fix: use subprocess.run() or call .wait().

  • fnull not closed (tests/integration/test_controller/test_controller.py:86-87) — open(os.devnull) not in with block.

Design / behavior

  • os.replace strips permissions (common/persist.py:75-81) — mkstemp creates 0600 files; os.replace transfers those permissions. Fix: preserve original file mode before replace.

  • no-enabled-pairs fallback logic (controller/controller.py:285-299) — When all pairs are disabled, no_enabled_pairs flag is set but a default pair still syncs. May be intentional — needs product decision.

  • os.path.join for remote POSIX paths (controller/scan/remote_scanner.py:45-47) — Uses host OS path separator for SSH remote paths. Fix: use posixpath.join. Low risk since app only runs in Linux Docker.

Not fixing (verified non-issues)

  • logger None in except (seedsync.py:478) — Logger is always initialized before run().
  • Joining unstarted threads (notifier.py:64) — Threads are only added after .start().
  • isdir check too broad (local_scanner.py:32) — Theoretical only.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestv0.14.3Targeted for v0.14.3 release

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions