From 99081afdbb3dc8c8839cdff57413fb133497bcc8 Mon Sep 17 00:00:00 2001 From: Kosta Vukicevic Date: Mon, 13 May 2024 11:33:00 +0200 Subject: [PATCH] feat!: Debug output for scheduled jobs (#1616) Activate debug output in schedule settings. Breaking Change: GUI started with --debug does no longer add --debug to the crontab for scheduled profiles by default. Additionally some refactoring and also pylint tweaks. Thanks to Kosta Vukicevic (@stcksmsh) for submitting this PR. Close #1616 --------- Co-authored-by: Christian Buhtz Co-authored-by: aryoda <11374410+aryoda@users.noreply.github.com> --- CHANGES | 5 +- FAQ.md | 2 +- common/config.py | 46 ++++++++++++-- common/snapshots.py | 6 +- common/test/test_crontab.py | 104 ++++++++++++++++++++++++++++++++ common/test/test_diagnostics.py | 2 +- common/test/test_lint.py | 34 ++++++++++- common/test/test_tools.py | 2 +- common/tools.py | 39 +++++++----- qt/app.py | 4 -- qt/plugins/systrayiconplugin.py | 4 +- qt/qttools_path.py | 1 - qt/restoredialog.py | 1 - qt/settingsdialog.py | 18 +++++- qt/test/test_lint.py | 32 +++++++++- 15 files changed, 259 insertions(+), 41 deletions(-) create mode 100644 common/test/test_crontab.py diff --git a/CHANGES b/CHANGES index fdf78d386..0384d066d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ Back In Time Version 1.4.4-dev (development of upcoming release) +* Breaking Change: GUI started with --debug does no longer add --debug to the crontab for scheduled profiles. + Use the new "enable logging for debug messages" in the 'Schedule' section of the 'Manage profiles' GUI instead. +* Feature: Profile and GUI allow to activate debug output for scheduled jobs by adding '--debug' to crontab entry (#1616, contributed by @stcksmsh Kosta Vukicevic) * Removed: Field "filesystem_mount" and "snapshot_version" in "info" file (#1684) * Feature: Support SSH proxy (jump) host (#1688) (@cgrinham, Christie Grinham) * Removed: Context menu in LogViewDialog (#1578) @@ -8,7 +11,7 @@ Version 1.4.4-dev (development of upcoming release) * Fix: Validation of diff command settings in compare snapshots dialog (#1662) (@stcksmsh Kosta Vukicevic) * Fix bug: Open symlinked folders in file view (#1476) * Fix bug: Respect dark mode using color roles (#1601) -* Fix: "Highly recommended" exclusion pattern in "Manage Profile" dialog's "Exclude" tab show missings only (#1620) +* Fix: "Highly recommended" exclusion pattern in "Manage Profile" dialog's "Exclude" tab show missing only (#1620) * Build: Activate PyLint error E0401 (import-error) * Dependency: Migration to PyQt6 * Build: PyLint unit test is skipped if PyLint isn't installed, but will always run on TravisCI (#1634) diff --git a/FAQ.md b/FAQ.md index ff4b7cc6a..1536eda5a 100644 --- a/FAQ.md +++ b/FAQ.md @@ -458,7 +458,7 @@ Otherwise, kill the process. After that look into the folder For more details see the developer documentation: [Usage of control files (locks, flocks, logs and others)](common/doc-dev/4_Control_files_usage_(locks_flocks_logs_and_others).md) ### Switching to dark or light mode in the desktop environment is ignored by BIT -After restart _Back In Time_ it should addapt to the desktops current used +After restart _Back In Time_ it should adapt to the desktops current used color theme. It happens because Qt does not detect theme modifications out of the diff --git a/common/config.py b/common/config.py index 2d46dccfd..bd216d674 100644 --- a/common/config.py +++ b/common/config.py @@ -989,6 +989,13 @@ def scheduleMode(self, profile_id = None): def setScheduleMode(self, value, profile_id = None): self.setProfileIntValue('schedule.mode', value, profile_id) + def scheduleDebug(self, profile_id = None): + #?Enable debug output to system log for schedule mode. + return self.profileBoolValue('schedule.debug', False, profile_id) + + def setScheduleDebug(self, value, profile_id = None): + self.setProfileBoolValue('schedule.debug', value, profile_id) + def scheduleTime(self, profile_id = None): #?Position-coded number with the format "hhmm" to specify the hour #?and minute the cronjob should start (eg. 2015 means a quarter @@ -1787,28 +1794,59 @@ def cronLine(self, profile_id): return cron_line def cronCmd(self, profile_id): - if not tools.checkCommand('backintime'): - logger.error("Command 'backintime' not found", self) - return + """Generates the command used in the crontab file based on the settings + for the current profile. + + Returns: + str: The crontab line. + """ + + # buhtz (2024-04): IMHO meaningless in productive environments. + # if not tools.checkCommand('backintime'): + # logger.error("Command 'backintime' not found", self) + # return + + # Get full path of the Back In Time binary cmd = tools.which('backintime') + ' ' + + # The "--profile-id" argument is used only for profiles different from + # first profile if profile_id != '1': cmd += '--profile-id %s ' % profile_id + + # User defined path to config file if not self._LOCAL_CONFIG_PATH is self._DEFAULT_CONFIG_PATH: cmd += '--config %s ' % self._LOCAL_CONFIG_PATH - if logger.DEBUG: + + # Enable debug output + if self.scheduleDebug(profile_id): cmd += '--debug ' + + # command cmd += 'backup-job' + + # Redirect stdout to nirvana if self.redirectStdoutInCron(profile_id): cmd += ' >/dev/null' + + # Redirect stderr ... if self.redirectStderrInCron(profile_id): + if self.redirectStdoutInCron(profile_id): + # ... to stdout cmd += ' 2>&1' else: + # ... to nirvana cmd += ' 2>/dev/null' + + # IO priority: low (-n7) in "best effort" class (-c2) if self.ioniceOnCron(profile_id) and tools.checkCommand('ionice'): cmd = tools.which('ionice') + ' -c2 -n7 ' + cmd + + # CPU priority: very low if self.niceOnCron(profile_id) and tools.checkCommand('nice'): cmd = tools.which('nice') + ' -n19 ' + cmd + return cmd diff --git a/common/snapshots.py b/common/snapshots.py index 3bf7cb0a0..704d86c95 100644 --- a/common/snapshots.py +++ b/common/snapshots.py @@ -52,7 +52,7 @@ class Snapshots: the class `SID` which represents a snapshot in the "data layer". BUHTZ 2024-02-23: Not sure but it seems to be one concret snapshot and - not a collection of snapshots. In this case the class name is missleading + not a collection of snapshots. In this case the class name is misleading because it is in plural form. Args: @@ -103,7 +103,7 @@ def takeSnapshotMessage(self): Dev note (buhtz): Too many try..excepts in here. """ - # Dev note (buhtz): Not sure what happens here or why this is usefull. + # Dev note (buhtz): Not sure what happens here or why this is useful. wait = datetime.datetime.now() - datetime.timedelta(seconds=5) if self.lastBusyCheck < wait: @@ -172,7 +172,7 @@ def setTakeSnapshotMessage(self, type_id, message, timeout=-1): message_fn = self.config.takeSnapshotMessageFile() try: - # Write message to file (and overwrites the previos one) + # Write message to file (and overwrites the previous one) with open(message_fn, 'wt') as f: f.write(str(type_id) + '\n' + message) diff --git a/common/test/test_crontab.py b/common/test/test_crontab.py new file mode 100644 index 000000000..9c8f72482 --- /dev/null +++ b/common/test/test_crontab.py @@ -0,0 +1,104 @@ +# Back In Time +# Copyright (C) 2024 Kosta Vukicevic, Christian Buhtz +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation,Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +import unittest +import pyfakefs.fake_filesystem_unittest as pyfakefs_ut +import sys +import os +import tempfile +import inspect +from pathlib import Path +from unittest import mock +sys.path.append(os.path.join(os.path.dirname(__file__), '..')) +import backintime +import config +import snapshots +import tools +import logger + + +class CrontabDebug(pyfakefs_ut.TestCase): + """Debug behavior when scheduled via crontab""" + def setUp(self): + """Setup a fake filesystem with a config file.""" + self.setUpPyfakefs(allow_root_user=False) + + # cleanup() happens automatically + self._temp_dir = tempfile.TemporaryDirectory(prefix='bit.') + # Workaround: tempfile and pathlib not compatible yet + self.temp_path = Path(self._temp_dir.name) + + self.config_fp = self._create_config_file(parent_path=self.temp_path) + + def _create_config_file(cls, parent_path): + """Minimal config file""" + cfg_content = inspect.cleandoc(''' + config.version=6 + profile1.snapshots.include.1.type=0 + profile1.snapshots.include.1.value=rootpath/source + profile1.snapshots.include.size=1 + profile1.snapshots.no_on_battery=false + profile1.snapshots.notify.enabled=true + profile1.snapshots.path=rootpath/destination + profile1.snapshots.path.host=test-host + profile1.snapshots.path.profile=1 + profile1.snapshots.path.user=test-user + profile1.snapshots.preserve_acl=false + profile1.snapshots.preserve_xattr=false + profile1.snapshots.remove_old_snapshots.enabled=true + profile1.snapshots.remove_old_snapshots.unit=80 + profile1.snapshots.remove_old_snapshots.value=10 + profile1.snapshots.rsync_options.enabled=false + profile1.snapshots.rsync_options.value= + profiles.version=1 + ''') + + # config file location + config_fp = parent_path / 'config_path' / 'config' + config_fp.parent.mkdir() + config_fp.write_text(cfg_content, 'utf-8') + + return config_fp + + @mock.patch('tools.which', return_value='backintime') + def test_crontab_contains_debug(self, mock_which): + """ + About mock_which: A workaround because the function gives + false-negative when using a fake filesystem. + """ + conf = config.Config(str(self.config_fp)) + + # set and assert start conditions + conf.setScheduleDebug(True) + self.assertTrue(conf.scheduleDebug()) + + sut = conf.cronCmd(profile_id='1') + self.assertIn('--debug', sut) + + @mock.patch('tools.which', return_value='backintime') + def test_crontab_without_debug(self, mock_which): + """No debug output in crontab line. + + About mock_which: See test_crontab_with_debug(). + """ + conf = config.Config(str(self.config_fp)) + + # set and assert start conditions + conf.setScheduleDebug(False) + self.assertFalse(conf.scheduleDebug()) + + sut = conf.cronCmd(profile_id='1') + self.assertNotIn('--debug', sut) diff --git a/common/test/test_diagnostics.py b/common/test/test_diagnostics.py index e611d1664..4168f12fa 100644 --- a/common/test/test_diagnostics.py +++ b/common/test/test_diagnostics.py @@ -28,7 +28,7 @@ def test_content_minimal(self): self.assertCountEqual(sut['host-setup'].keys(), ['OS']) def test_some_content(self): - """Some containted elements""" + """Some contained elements""" result = diagnostics.collect_diagnostics() # 1st level keys diff --git a/common/test/test_lint.py b/common/test/test_lint.py index c85503bef..4e4524ad0 100644 --- a/common/test/test_lint.py +++ b/common/test/test_lint.py @@ -65,22 +65,49 @@ def test_with_pylint(self): # Deactivate all checks by default '--disable=all', # prevent false-positive no-module-member errors - '--extension-pkg-whitelist=PyQt6,PyQt6.QtCore', + '--extension-pkg-allow-list=PyQt6,PyQt6.QtCore', # Because of globally installed GNU gettext functions '--additional-builtins=_,ngettext', # PEP8 conform line length (see PyLint Issue #3078) '--max-line-length=79', # Whitelist variable names '--good-names=idx,fp', + # '--reports=yes', ] # Explicit activate checks err_codes = [ + 'E0401', # import-error 'E0602', # undefined-variable 'E1101', # no-member + # 'W0611', # unused-import + 'W1301', # unused-format-string-key 'W1401', # anomalous-backslash-in-string (invalid escape sequence) - 'E0401', # import-error 'I0021', # useless-suppression + + # Enable asap. This list is selection of existing (not all!) + # problems currently exiting in the BIT code base. Quit easy to fix + # because there count is low. + # 'C0303', # trailing-whitespace + # 'C0305', # trailing-newlines + # 'C0324', # superfluous-parens + # 'C0410', # multiple-imports + # 'E0213', # no-self-argument + # 'R0201', # no-self-use + # 'R0202', # no-classmethod-decorator + # 'R0203', # no-staticmethod-decorator + # 'R0801', # duplicate-code + # 'W0123', # eval-used + # 'W0237', # arguments-renamed + # 'W0221', # arguments-differ + # 'W0311', # bad-indentation + # 'W0404', # reimported + # 'W4902', # deprecated-method + # 'W4904', # deprecated-class + # 'W0603', # global-statement + # 'W0614', # unused-wildcard-import + # 'W0612', # unused-variable + # 'W0707', # raise-missing-from ] if ON_TRAVIS_PPC64LE: @@ -105,3 +132,6 @@ def test_with_pylint(self): print(r.stdout) self.assertEqual(0, error_n, f'PyLint found {error_n} problems.') + + # any other errors? + self.assertEqual(r.stderr, '') diff --git a/common/test/test_tools.py b/common/test/test_tools.py index 0532dd1e9..7d630ade7 100644 --- a/common/test/test_tools.py +++ b/common/test/test_tools.py @@ -812,7 +812,7 @@ def setUp(self): self.setUpPyfakefs(allow_root_user=False) def test_git_repo_info_none(self): - """Acutally not a git repo""" + """Actually not a git repo""" self.assertEqual(tools.get_git_repository_info(), None) diff --git a/common/tools.py b/common/tools.py index 7533968a4..61fa96c5b 100644 --- a/common/tools.py +++ b/common/tools.py @@ -357,11 +357,15 @@ def registerBackintimePath(*path): def runningFromSource(): - """ - Check if BackInTime is running from source (without installing). + """Check if BackInTime is running from source (without installing). + + Dev notes by buhtz (2024-04): This function is dangerous and will give a + false-negative in fake filesystems (e.g. PyFakeFS). The function should + not exist. Beside unit tests it is used only two times. Remove it until + migration to pyproject.toml based project packaging (#1575). Returns: - bool: ``True`` if BackInTime is running from source + bool: ``True`` if BackInTime is running from source. """ return os.path.isfile(backintimePath('common', 'backintime')) @@ -493,14 +497,13 @@ def readFileLines(path, default = None): def checkCommand(cmd): - """ - Check if command ``cmd`` is a file in 'PATH' environ. + """Check if command ``cmd`` is a file in 'PATH' environment. Args: - cmd (str): command + cmd (str): The command. Returns: - bool: ``True`` if command ``cmd`` is in 'PATH' environ + bool: ``True`` if ``cmd`` is in 'PATH' environment otherwise ``False``. """ cmd = cmd.strip() @@ -510,23 +513,27 @@ def checkCommand(cmd): if os.path.isfile(cmd): return True - return not which(cmd) is None - + return which(cmd) is not None + def which(cmd): - """ - Get the fullpath of executable command ``cmd``. Works like - command-line 'which' command. + """Get the fullpath of executable command ``cmd``. + + Works like command-line 'which' command. + + Dev note by buhtz (2024-04): Give false-negative results in fake + filesystems. Quit often use in the whole code base. But not sure why + can we replace it with "which" from shell? Args: - cmd (str): command + cmd (str): The command. Returns: - str: fullpath of command ``cmd`` or ``None`` if command is - not available + str: Fullpath of command ``cmd`` or ``None`` if command is not + available. """ pathenv = os.getenv('PATH', '') - path = pathenv.split(":") + path = pathenv.split(':') common = backintimePath('common') if runningFromSource() and common not in path: diff --git a/qt/app.py b/qt/app.py index 07a3831d8..09d691140 100644 --- a/qt/app.py +++ b/qt/app.py @@ -52,7 +52,6 @@ QShortcut, QDesktopServices, QPalette, - QColor, QIcon, QFileSystemModel) from PyQt6.QtWidgets import (QWidget, @@ -69,7 +68,6 @@ QAbstractItemView, QStyledItemDelegate, QVBoxLayout, - QHBoxLayout, QStackedLayout, QSplitter, QGroupBox, @@ -79,7 +77,6 @@ QMessageBox, QInputDialog, QDialog, - QDialogButtonBox, QApplication, ) from PyQt6.QtCore import (Qt, @@ -92,7 +89,6 @@ QEvent, QSortFilterProxyModel, QDir, - QSize, QUrl, pyqtRemoveInputHook, ) diff --git a/qt/plugins/systrayiconplugin.py b/qt/plugins/systrayiconplugin.py index b6aeff675..e01a78e95 100644 --- a/qt/plugins/systrayiconplugin.py +++ b/qt/plugins/systrayiconplugin.py @@ -33,13 +33,11 @@ import pluginmanager import tools import logger -import time import gettext -import _thread import subprocess -_=gettext.gettext +_ = gettext.gettext if not os.getenv('DISPLAY', ''): diff --git a/qt/qttools_path.py b/qt/qttools_path.py index fd6b47641..1dd59eea2 100644 --- a/qt/qttools_path.py +++ b/qt/qttools_path.py @@ -25,7 +25,6 @@ """ import os import sys -import gettext def backintimePath(*path): return os.path.abspath(os.path.join(__file__, os.pardir, os.pardir, *path)) diff --git a/qt/restoredialog.py b/qt/restoredialog.py index 383a5df76..597ea56ba 100644 --- a/qt/restoredialog.py +++ b/qt/restoredialog.py @@ -23,7 +23,6 @@ from PyQt6.QtCore import * import tools -import qttools class RestoreDialog(QDialog): diff --git a/qt/settingsdialog.py b/qt/settingsdialog.py index c5ab01d2a..cf43a1aa6 100644 --- a/qt/settingsdialog.py +++ b/qt/settingsdialog.py @@ -22,7 +22,6 @@ import copy import re import getpass -import textwrap from PyQt6.QtGui import (QIcon, QFont, QPalette, @@ -562,6 +561,19 @@ def __init__(self, parent): self.comboSchedule.currentIndexChanged.connect(self.scheduleChanged) + self.cbScheduleDebug = QCheckBox(self) + self.cbScheduleDebug.setText(_('Enable logging of debug messages')) + qttools.set_wrapped_tooltip( + self.cbScheduleDebug, + [ + _('Writes debug-level messages into the system log via ' + '"--debug".'), + _('Caution: Only use this temporarily for diagnostics, as it ' + 'generates a large amount of output.') + ] + ) + glayout.addWidget(self.cbScheduleDebug, 8, 0) + # layout.addStretch() scrollArea.setWidget(layoutWidget) @@ -1434,6 +1446,8 @@ def updateProfile(self): self.config.scheduleRepeatedUnit()) self.updateSchedule(self.config.scheduleMode()) + self.cbScheduleDebug.setChecked(self.config.scheduleDebug()) + # TAB: Include self.listInclude.clear() @@ -1674,6 +1688,8 @@ def saveProfile(self): self.comboScheduleRepeatedUnit.itemData( self.comboScheduleRepeatedUnit.currentIndex())) + self.config.setScheduleDebug(self.cbScheduleDebug.isChecked()) + # auto-remove self.config.setRemoveOldSnapshots( self.cbRemoveOlder.isChecked(), diff --git a/qt/test/test_lint.py b/qt/test/test_lint.py index c85503bef..22e6eab06 100644 --- a/qt/test/test_lint.py +++ b/qt/test/test_lint.py @@ -65,22 +65,50 @@ def test_with_pylint(self): # Deactivate all checks by default '--disable=all', # prevent false-positive no-module-member errors - '--extension-pkg-whitelist=PyQt6,PyQt6.QtCore', + '--extension-pkg-allow-list=PyQt6,PyQt6.QtCore', # Because of globally installed GNU gettext functions '--additional-builtins=_,ngettext', # PEP8 conform line length (see PyLint Issue #3078) '--max-line-length=79', # Whitelist variable names '--good-names=idx,fp', + # '--reports=yes', ] # Explicit activate checks err_codes = [ + 'E0401', # import-error 'E0602', # undefined-variable 'E1101', # no-member + 'W0611', # unused-import + 'W1301', # unused-format-string-key 'W1401', # anomalous-backslash-in-string (invalid escape sequence) - 'E0401', # import-error 'I0021', # useless-suppression + + # Enable asap. This list is selection of existing (not all!) + # problems currently exiting in the BIT code base. Quit easy to fix + # because there count is low. + # 'C0303', # trailing-whitespace + # 'C0305', # trailing-newlines + # 'C0324', # superfluous-parens + # 'C0410', # multiple-imports + # 'E0213', # no-self-argument + # 'R0201', # no-self-use + # 'R0202', # no-classmethod-decorator + # 'R0203', # no-staticmethod-decorator + # 'R0801', # duplicate-code + # 'W0123', # eval-used + # 'W0237', # arguments-renamed + # 'W0221', # arguments-differ + # 'W0311', # bad-indentation + # 'W0404', # reimported + # 'W4902', # deprecated-method + # 'W4904', # deprecated-class + # 'W0603', # global-statement + # 'W0614', # unused-wildcard-import + # 'W0611', # unused-import + # 'W0612', # unused-variable + # 'W0707', # raise-missing-from ] if ON_TRAVIS_PPC64LE: