Random cleanups by @ThomasWaldmann (#1879)

* fix PEP8 E721

do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`

* remove redundant parentheses

* fix SiteWorker.run for empty job queue

local variable job is not assigned if queue was empty
when calling .run(), but it is used in exception handler.

* remove unreachable code in parse_diff_lines

* bug fix for unreachable code in is_worker_running

the code intended to check if *any* worker is running for
any site was *unreachable*.

this caused false negative results for site=None.

* check_failed_response: remove outdated part of docstring

* pull request template: fix relative path to LICENSE.txt

* fix typos

* use logger.warning, .warn is deprecated
This commit is contained in:
TW 2024-01-09 09:06:48 +01:00 committed by GitHub
parent 1f062359d8
commit 675010e401
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 43 additions and 46 deletions

View File

@ -1,5 +1,5 @@
name: "Bug Report Form" name: "Bug Report Form"
description: "Report a bug or a similiar issue." description: "Report a bug or a similar issue."
body: body:
- type: markdown - type: markdown
attributes: attributes:

View File

@ -1,6 +1,6 @@
--- ---
name: Bug Report name: Bug Report
about: Report a bug or a similiar issue - the classic way about: Report a bug or a similar issue - the classic way
title: '' title: ''
labels: '' labels: ''
assignees: '' assignees: ''
@ -18,7 +18,7 @@ If you want to suggest a feature or have any other question, please use our
#### Description #### Description
<!-- Description <!-- Description
Please decribe your issue and its context in a clear and concise way. Please describe your issue and its context in a clear and concise way.
Please try to reproduce the issue and provide the steps to reproduce it. Please try to reproduce the issue and provide the steps to reproduce it.
--> -->

View File

@ -36,7 +36,7 @@
- [ ] All new and existing tests passed. - [ ] All new and existing tests passed.
*I provide my contribution under the terms of the [license](./../../LICENSE.txt) of this repository and I affirm the [Developer Certificate of Origin][dco].* *I provide my contribution under the terms of the [license](./../LICENSE.txt) of this repository and I affirm the [Developer Certificate of Origin][dco].*
[dco]: https://developercertificate.org/ [dco]: https://developercertificate.org/

View File

@ -28,10 +28,10 @@ else:
@nox.parametrize("borgbackup", supported_borgbackup_versions) @nox.parametrize("borgbackup", supported_borgbackup_versions)
def run_tests(session, borgbackup): def run_tests(session, borgbackup):
# install borgbackup # install borgbackup
if (sys.platform == 'darwin'): if sys.platform == 'darwin':
# in macOS there's currently no fuse package which works with borgbackup directly # in macOS there's currently no fuse package which works with borgbackup directly
session.install(f"borgbackup=={borgbackup}") session.install(f"borgbackup=={borgbackup}")
elif (borgbackup == "1.1.18"): elif borgbackup == "1.1.18":
# borgbackup 1.1.18 doesn't support pyfuse3 # borgbackup 1.1.18 doesn't support pyfuse3
session.install("llfuse") session.install("llfuse")
session.install(f"borgbackup[llfuse]=={borgbackup}") session.install(f"borgbackup[llfuse]=={borgbackup}")

View File

@ -18,10 +18,10 @@ def create_symlink(folder: Path) -> None:
"""Create the appropriate symlink in the MacOS folder """Create the appropriate symlink in the MacOS folder
pointing to the Resources folder. pointing to the Resources folder.
""" """
sibbling = Path(str(folder).replace("MacOS", "")) sibling = Path(str(folder).replace("MacOS", ""))
# PyQt6/Qt/qml/QtQml/Models.2 # PyQt6/Qt/qml/QtQml/Models.2
root = str(sibbling).partition("Contents")[2].lstrip("/") root = str(sibling).partition("Contents")[2].lstrip("/")
# ../../../../ # ../../../../
backward = "../" * (root.count("/") + 1) backward = "../" * (root.count("/") + 1)
# ../../../../Resources/PyQt6/Qt/qml/QtQml/Models.2 # ../../../../Resources/PyQt6/Qt/qml/QtQml/Models.2
@ -41,7 +41,7 @@ def fix_dll(dll: Path) -> None:
def match_func(pth: str) -> Optional[str]: def match_func(pth: str) -> Optional[str]:
"""Callback function for MachO.rewriteLoadCommands() that is """Callback function for MachO.rewriteLoadCommands() that is
called on every lookup path setted in the DLL headers. called on every lookup path set in the DLL headers.
By returning None for system libraries, it changes nothing. By returning None for system libraries, it changes nothing.
Else we return a relative path pointing to the good file Else we return a relative path pointing to the good file
in the MacOS folder. in the MacOS folder.
@ -73,7 +73,7 @@ def find_problematic_folders(folder: Path) -> Generator[Path, None, None]:
"""Recursively yields problematic folders (containing a dot in their name).""" """Recursively yields problematic folders (containing a dot in their name)."""
for path in folder.iterdir(): for path in folder.iterdir():
if not path.is_dir() or path.is_symlink(): if not path.is_dir() or path.is_symlink():
# Skip simlinks as they are allowed (even with a dot) # Skip symlinks as they are allowed (even with a dot)
continue continue
if "." in path.name: if "." in path.name:
yield path yield path
@ -83,7 +83,7 @@ def find_problematic_folders(folder: Path) -> Generator[Path, None, None]:
def move_contents_to_resources(folder: Path) -> Generator[Path, None, None]: def move_contents_to_resources(folder: Path) -> Generator[Path, None, None]:
"""Recursively move any non symlink file from a problematic folder """Recursively move any non symlink file from a problematic folder
to the sibbling one in Resources. to the sibling one in Resources.
""" """
for path in folder.iterdir(): for path in folder.iterdir():
if path.is_symlink(): if path.is_symlink():
@ -91,10 +91,10 @@ def move_contents_to_resources(folder: Path) -> Generator[Path, None, None]:
if path.name == "qml": if path.name == "qml":
yield from move_contents_to_resources(path) yield from move_contents_to_resources(path)
else: else:
sibbling = Path(str(path).replace("MacOS", "Resources")) sibling = Path(str(path).replace("MacOS", "Resources"))
sibbling.parent.mkdir(parents=True, exist_ok=True) sibling.parent.mkdir(parents=True, exist_ok=True)
shutil.move(path, sibbling) shutil.move(path, sibling)
yield sibbling yield sibling
def main(args: List[str]) -> int: def main(args: List[str]) -> int:

View File

@ -308,11 +308,6 @@ class VortaApp(QtSingleApplication):
Displays a `QMessageBox` with an error message depending on the Displays a `QMessageBox` with an error message depending on the
return code of the `BorgJob`. return code of the `BorgJob`.
Parameters
----------
repo_url : str
The url of the repo of concern
""" """
# extract data from the params for the borg job # extract data from the params for the borg job
repo_url = result['params']['repo_url'] repo_url = result['params']['repo_url']
@ -344,7 +339,7 @@ class VortaApp(QtSingleApplication):
elif returncode > 128: elif returncode > 128:
# 128+N - killed by signal N (e.g. 137 == kill -9) # 128+N - killed by signal N (e.g. 137 == kill -9)
signal = returncode - 128 signal = returncode - 128
text = self.tr('Repository data check for repo was killed by signal %s.') % (signal) text = self.tr('Repository data check for repo was killed by signal %s.') % signal
infotext = self.tr('The process running the check job got a kill signal. Try again.') infotext = self.tr('The process running the check job got a kill signal. Try again.')
else: else:
# Real error # Real error

View File

@ -25,9 +25,9 @@ class JobInterface(QObject):
@abstractmethod @abstractmethod
def cancel(self): def cancel(self):
""" """
Cancel can be called when the job is not started. It is the responsability of FuncJob to not cancel job if Cancel can be called when the job is not started. It is the responsibility of FuncJob to not cancel job if
no job is running. no job is running.
The cancel mehod of JobsManager calls the cancel method on the running jobs only. Other jobs are dequeued. The cancel method of JobsManager calls the cancel method on the running jobs only. Other jobs are dequeued.
""" """
pass pass
@ -50,6 +50,7 @@ class SiteWorker(threading.Thread):
self.current_job = None self.current_job = None
def run(self): def run(self):
job = None
while True: while True:
try: try:
job = self.jobs.get(False) job = self.jobs.get(False)
@ -58,7 +59,8 @@ class SiteWorker(threading.Thread):
job.run() job.run()
logger.debug("Finish job for site: %s", job.repo_id()) logger.debug("Finish job for site: %s", job.repo_id())
except queue.Empty: except queue.Empty:
logger.debug("No more jobs for site: %s", job.repo_id()) if job is not None:
logger.debug("No more jobs for site: %s", job.repo_id())
return return
@ -77,19 +79,20 @@ class JobsManager:
def is_worker_running(self, site=None): def is_worker_running(self, site=None):
""" """
See if there are any active jobs. The user can't start a backup if a job is See if there are any active jobs.
running. The scheduler can. The user can't start a backup if a job is running. The scheduler can.
"""
# Check status for specific site (repo)
if site in self.workers:
return self.workers[site].is_alive()
else:
return False
# Check if *any* worker is active If site is None, check if there is any worker active for any site (repo).
for _, worker in self.workers.items(): If site is not None, only check if there is a worker active for the given site (repo).
if worker.is_alive(): """
return True if site is not None:
if site in self.workers:
if self.workers[site].is_alive():
return True
else:
for _, worker in self.workers.items():
if worker.is_alive():
return True
return False return False
def add_job(self, job): def add_job(self, job):

View File

@ -24,7 +24,7 @@ class NetworkStatusMonitor:
def is_network_status_available(self): def is_network_status_available(self):
"""Is the network status really available, and not just a dummy implementation?""" """Is the network status really available, and not just a dummy implementation?"""
return type(self) != NetworkStatusMonitor return type(self) is not NetworkStatusMonitor
def is_network_metered(self) -> bool: def is_network_metered(self) -> bool:
"""Is the currently connected network a metered connection?""" """Is the currently connected network a metered connection?"""

View File

@ -36,7 +36,7 @@ class ProfileExport:
def repo_url(self): def repo_url(self):
if ( if (
'repo' in self._profile_dict 'repo' in self._profile_dict
and type(self._profile_dict['repo']) == dict and isinstance(self._profile_dict['repo'], dict)
and 'url' in self._profile_dict['repo'] and 'url' in self._profile_dict['repo']
): ):
return self._profile_dict['repo']['url'] return self._profile_dict['repo']['url']

View File

@ -70,7 +70,7 @@ class VortaScheduler(QtCore.QObject):
self.bus = bus self.bus = bus
self.bus.connect(service, path, interface, name, "b", self.loginSuspendNotify) self.bus.connect(service, path, interface, name, "b", self.loginSuspendNotify)
else: else:
logger.warn('Failed to connect to DBUS interface to detect sleep/resume events') logger.warning('Failed to connect to DBUS interface to detect sleep/resume events')
@QtCore.pyqtSlot(bool) @QtCore.pyqtSlot(bool)
def loginSuspendNotify(self, suspend: bool): def loginSuspendNotify(self, suspend: bool):

View File

@ -875,7 +875,7 @@ class ArchiveTab(ArchiveTabBase, ArchiveTabUI, BackupProfileMixin):
return msg.exec() == QMessageBox.StandardButton.Yes return msg.exec() == QMessageBox.StandardButton.Yes
def delete_action(self): def delete_action(self):
# Since this function modify the UI, we can't put the whole function in a JobQUeue. # Since this function modify the UI, we can't put the whole function in a JobQueue.
# determine selected archives # determine selected archives
archives = [] archives = []

View File

@ -381,7 +381,6 @@ def parse_diff_lines(lines: List[str], model: 'DiffTree'):
if not parsed_line: if not parsed_line:
raise Exception("Couldn't parse diff output `{}`".format(line)) raise Exception("Couldn't parse diff output `{}`".format(line))
continue
path = PurePath(parsed_line['path']) path = PurePath(parsed_line['path'])
file_type = FileType.FILE file_type = FileType.FILE

View File

@ -610,7 +610,7 @@ class FileTreeModel(QAbstractItemModel, Generic[T]):
if isinstance(path, PurePath): if isinstance(path, PurePath):
path = path.parts path = path.parts
return self.root.get_path(path) # handels empty path return self.root.get_path(path) # handles empty path
def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole): def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole):
""" """

View File

@ -27,7 +27,7 @@ class AddProfileWindow(AddProfileBase, AddProfileUI):
self.name_blank = trans_late('AddProfileWindow', 'Please enter a profile name.') self.name_blank = trans_late('AddProfileWindow', 'Please enter a profile name.')
self.name_exists = trans_late('AddProfileWindow', 'A profile with this name already exists.') self.name_exists = trans_late('AddProfileWindow', 'A profile with this name already exists.')
# Call validate to set inital messages # Call validate to set initial messages
self.buttonBox.button(QDialogButtonBox.StandardButton.Save).setEnabled(self.validate()) self.buttonBox.button(QDialogButtonBox.StandardButton.Save).setEnabled(self.validate())
def _set_status(self, text): def _set_status(self, text):

View File

@ -40,7 +40,7 @@ class RepoTab(RepoBase, RepoUI, BackupProfileMixin):
# compression or speed on a unified scale. this is not 1-dimensional and also depends # compression or speed on a unified scale. this is not 1-dimensional and also depends
# on the input data. so we just tell what we know for sure. # on the input data. so we just tell what we know for sure.
# "auto" is used for some slower / older algorithms to avoid wasting a lot of time # "auto" is used for some slower / older algorithms to avoid wasting a lot of time
# on uncompressible data. # on incompressible data.
self.repoCompression.addItem(self.tr('LZ4 (modern, default)'), 'lz4') self.repoCompression.addItem(self.tr('LZ4 (modern, default)'), 'lz4')
self.repoCompression.addItem(self.tr('Zstandard Level 3 (modern)'), 'zstd,3') self.repoCompression.addItem(self.tr('Zstandard Level 3 (modern)'), 'zstd,3')
self.repoCompression.addItem(self.tr('Zstandard Level 8 (modern)'), 'zstd,8') self.repoCompression.addItem(self.tr('Zstandard Level 8 (modern)'), 'zstd,8')

View File

@ -331,7 +331,7 @@ class SourceTab(SourceBase, SourceUI, BackupProfileMixin):
profile = self.profile() profile = self.profile()
# sort indexes, starting with lowest # sort indexes, starting with lowest
indexes.sort() indexes.sort()
# remove each selected row, starting with highest index (otherways, higher indexes become invalid) # remove each selected row, starting with the highest index (otherwise, higher indexes become invalid)
for index in reversed(indexes): for index in reversed(indexes):
db_item = SourceFileModel.get( db_item = SourceFileModel.get(
dir=self.sourceFilesWidget.item(index.row(), SourceColumn.Path).text(), dir=self.sourceFilesWidget.item(index.row(), SourceColumn.Path).text(),

View File

@ -87,7 +87,7 @@ class TestFileSystemItem:
item.add(child2) item.add(child2)
item.add(child3) item.add(child3)
# test get inexistent subpath # test get nonexistent subpath
assert item.get('unknown') is None assert item.get('unknown') is None
assert item.get('unknown', default='default') == 'default' assert item.get('unknown', default='default') == 'default'