borgmatic-collective/borgmatic
borgmatic-collective
/
borgmatic
14
58
Fork 41
Code Issues 55 Pull Requests 2 Actions Packages Releases 122 Activity

WIP: Added debug log entries for borg, borgmatic and os version #750

Closed
IBims1NicerTobi wants to merge 2 commits from (deleted):main into main
pull from: (deleted):main
merge into: borgmatic-collective:main
Conversation 7 Commits 2 Files Changed 2 +45
2 changed files with 45 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
borgmatic/commands/borgmatic.py
View File

@ -36,6 +36,7 @@ import borgmatic.actions.rlist
import borgmatic.actions.transfer
import borgmatic.commands.completion.bash
import borgmatic.commands.completion.fish
from borgmatic.debug_env import log_versions
from borgmatic.borg import umount as borg_umount
from borgmatic.borg import version as borg_version
from borgmatic.commands.arguments import parse_arguments
@ -73,6 +74,7 @@ def run_configuration(config_filename, config, arguments):
try:
local_borg_version = borg_version.local_borg_version(config, local_path)
yield from log_error_records(f'{config_filename}: Local Borg version: {local_borg_version}', levelno=logging.DEBUG)
witten commented 2023-09-06 05:02:28 +00:00
Review
Copy Link

Is there any reason this is done here rather than passing local_borg_version to log_versions()? That would ensure it'd get logged in the same spot as all of your other nice debug log statements.

Is there any reason this is done here rather than passing `local_borg_version` to `log_versions()`? That would ensure it'd get logged in the same spot as all of your other nice debug log statements.
IBims1NicerTobi commented 2023-09-06 09:57:17 +00:00
Review
Copy Link

The borg version depends on the config file and run_configuration() is called later than log_versions(). Also there is the possibility of having one borg version for each config file, so we cannot really easily collect that in log_versions().

The borg version depends on the config file and `run_configuration()` is called later than `log_versions()`. Also there is the possibility of having one borg version for each config file, so we cannot really easily collect that in `log_versions()`.
witten commented 2023-09-06 17:13:56 +00:00
Review
Copy Link

Makes sense! Thanks for the explanation.

Makes sense! Thanks for the explanation.
except (OSError, CalledProcessError, ValueError) as error:
yield from log_error_records(f'{config_filename}: Error getting local Borg version', error)
return
@ -865,6 +867,8 @@ def main(extra_summary_logs=[]): # pragma: no cover
logger.critical(f'Error configuring logging: {error}')
exit_with_help_link()
log_versions()
summary_logs = (
extra_summary_logs
+ parse_logs

41
borgmatic/debug_env.py Normal file
View File

@ -0,0 +1,41 @@
import platform
import logging
import os
from borgmatic.borg import version as borg_version
try:
import importlib_metadata
except ModuleNotFoundError: # pragma: nocover
import importlib.metadata as importlib_metadata
logger = logging.Logger(__name__)
def log_versions(): # pragma: nocover
'''
Logs the platform and the architecture on debug. Also logs the version of borgmatic and python.
'''
logging.debug(f'Borgmatic version: {importlib_metadata.version("borgmatic")}')
witten commented 2023-09-06 04:50:32 +00:00
Review
Copy Link

borgmatic is always lowercase! (It's a convention often followed by command-line Unix tools.)

borgmatic is always lowercase! (It's a convention often followed by command-line Unix tools.)
IBims1NicerTobi commented 2023-09-06 10:01:07 +00:00
Review
Copy Link

Will be fixed

Will be fixed
logging.debug(f'Architecture: {platform.machine()}')
logging.debug(f'Python: {platform.python_implementation()} {platform.python_version()}')
if platform.system() == 'Linux':
logging.debug('Operating system: Linux')
logging.debug(f'Kernel: {os.uname()[2]}')
try:
os_release = platform.freedesktop_os_release()
if 'PRETTY_NAME' in os_release:
logging.debug(f'Distribution: {os_release["PRETTY_NAME"]}')
elif 'NAME' in os_release:
logging.debug(f'Distribution name: {os_release["NAME"]}')
if 'VERSION' in os_release:
logging.debug(f'Distribution version: {os_release["VERSION"]}')
witten commented 2023-09-06 04:56:48 +00:00
Review
Copy Link

What do you think of just logging the following when 'NAME' in os_release:?

logging.debug('Distribution: {os_release["NAME"]} {os_release.get("VERSION", "")}')

That'd be more comparable to the PRETTY_NAME case.

What do you think of just logging the following when `'NAME' in os_release:`? ```python logging.debug('Distribution: {os_release["NAME"]} {os_release.get("VERSION", "")}') ``` That'd be more comparable to the `PRETTY_NAME` case.
IBims1NicerTobi commented 2023-09-06 10:01:15 +00:00
Review
Copy Link

lgtm

lgtm
except OSError:
logging.debug('Cannot determine linux distribution: /etc/os-release and /usr/lib/os-release not accessible')
elif platform.system() == 'Darwin':
logging.debug('Operating system: MacOS')
logging.debug(f'Operating system version: {platform.mac_ver()[0]}')
elif platform.system() == 'Windows':
logging.debug('Operating system: Windows')
logging.debug(f'Operating system version: {platform.win32_edition()[0]}')
else:
logging.debug(f'Operating system: {platform.system()}')