[checkmk-commits] Removed a piggyback-related import cycle.

Module: check_mk Branch: master Commit: 072aaaea9696c33ba638dedd4c7279b43384d436 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=072aaaea9696c3… Author: Sven Panne <sp(a)mathias-kettner.de> Date: Tue Jun 12 10:39:35 2018 +0200 Removed a piggyback-related import cycle. Although this removes a cycle, pylint now shows 7 cycles: For some obscure reason it finds 2 more cycles now, but those are not related to this commit. We really need to investigate what's goning on here, but to do this we should really, really disentangle pytest and pylint, the current state is totally arcane and not comprehensible at all... :-/ Back to the commit itself: To break the cycle, we thread through a parameter, although this is a bit verbose and breaks the piggyback "abstraction" a bit. Nevertheless, breaking dependency cycles is *far* more important than this tiny ugliness, and what we really want here is a class: This would fix both problems at once, and probably many more... Change-Id: I890f8d5666f5dd4551a2c52ae00fa7d05e8eb807 --- cmk_base/check_table.py | 2 +- cmk_base/config.py | 2 +- cmk_base/data_sources/piggyback.py | 16 ++++++++++------ cmk_base/modes/check_mk.py | 4 ++-- cmk_base/piggyback.py | 37 ++++++++++++++++++------------------- 5 files changed, 32 insertions(+), 29 deletions(-) diff --git a/cmk_base/check_table.py b/cmk_base/check_table.py index cca9a11..9a737b5 100644 --- a/cmk_base/check_table.py +++ b/cmk_base/check_table.py @@ -84,7 +84,7 @@ def get_check_table(hostname, remove_duplicates=False, use_cache=True, if not config.is_snmp_host(hostname) and cmk_base.check_utils.is_snmp_check(checkname) and \ (not config.has_management_board(hostname) or config.management_protocol_of(hostname) != "snmp"): passed = False - if not config.is_tcp_host(hostname) and not piggyback.has_piggyback_raw_data(hostname) \ + if not config.is_tcp_host(hostname) and not piggyback.has_piggyback_raw_data(config.piggyback_max_cachefile_age, hostname) \ and cmk_base.check_utils.is_tcp_check(checkname): passed = False is_checkname_valid_cache[the_id] = passed diff --git a/cmk_base/config.py b/cmk_base/config.py index 6984ad2..08576a4 100644 --- a/cmk_base/config.py +++ b/cmk_base/config.py @@ -849,7 +849,7 @@ def is_ping_host(hostname): import cmk_base.piggyback as piggyback return not is_snmp_host(hostname) \ and not is_tcp_host(hostname) \ - and not piggyback.has_piggyback_raw_data(hostname) \ + and not piggyback.has_piggyback_raw_data(piggyback_max_cachefile_age, hostname) \ and not has_management_board(hostname) diff --git a/cmk_base/data_sources/piggyback.py b/cmk_base/data_sources/piggyback.py index 154bf2f..424f63c 100644 --- a/cmk_base/data_sources/piggyback.py +++ b/cmk_base/data_sources/piggyback.py @@ -26,26 +26,30 @@ import os -import cmk.paths +from cmk.paths import tmp_dir -import cmk_base.piggyback as piggyback +from cmk_base.config import piggyback_max_cachefile_age +from cmk_base.piggyback import get_piggyback_raw_data from .abstract import CheckMKAgentDataSource + +def _raw_data(name): + return get_piggyback_raw_data(piggyback_max_cachefile_age, name) + + class PiggyBackDataSource(CheckMKAgentDataSource): def id(self): return "piggyback" def describe(self): - path = os.path.join(cmk.paths.tmp_dir, "piggyback", self._hostname) + path = os.path.join(tmp_dir, "piggyback", self._hostname) return "Process piggyback data from %s" % path def _execute(self): - return piggyback.get_piggyback_raw_data(self._hostname) \ - + piggyback.get_piggyback_raw_data(self._ipaddress) - + return _raw_data(self._hostname) + _raw_data(self._ipaddress) def _get_raw_data(self): """Returns the current raw data of this data source diff --git a/cmk_base/modes/check_mk.py b/cmk_base/modes/check_mk.py index 243d8e5..8d9ab62 100644 --- a/cmk_base/modes/check_mk.py +++ b/cmk_base/modes/check_mk.py @@ -709,8 +709,8 @@ modes.register(Mode( # '----------------------------------------------------------------------' def mode_cleanup_piggyback(): - import cmk_base.piggyback - cmk_base.piggyback.cleanup_piggyback_files() + from cmk_base.piggyback import cleanup_piggyback_files + cleanup_piggyback_files(config.piggyback_max_cachefile_age) modes.register(Mode( long_option="cleanup-piggyback", diff --git a/cmk_base/piggyback.py b/cmk_base/piggyback.py index ecf6eef..faac4df 100644 --- a/cmk_base/piggyback.py +++ b/cmk_base/piggyback.py @@ -33,26 +33,25 @@ from cmk.exceptions import MKGeneralException import cmk_base.utils import cmk_base.console as console -import cmk_base.config as config -def get_piggyback_raw_data(hostname): +def get_piggyback_raw_data(piggyback_max_cachefile_age, hostname): output = "" if not hostname: return output - for sourcehost, file_path in _get_piggyback_files(hostname): + for sourcehost, file_path in _get_piggyback_files(piggyback_max_cachefile_age, hostname): console.verbose("Using piggyback raw data from host %s.\n" % sourcehost) output += file(file_path).read() return output -def has_piggyback_raw_data(hostname): - return _get_piggyback_files(hostname) != [] +def has_piggyback_raw_data(piggyback_max_cachefile_age, hostname): + return _get_piggyback_files(piggyback_max_cachefile_age, hostname) != [] -def _get_piggyback_files(hostname): +def _get_piggyback_files(piggyback_max_cachefile_age, hostname): """Gather a list of piggyback files to read for further processing. Please note that there may be multiple parallel calls executing the @@ -87,9 +86,9 @@ def _get_piggyback_files(hostname): continue # File might've been deleted. That's ok. # Skip piggyback files that are outdated at all - if file_age > config.piggyback_max_cachefile_age: + if file_age > piggyback_max_cachefile_age: console.verbose("Piggyback file %s is outdated (%d seconds too old). Skip processing.\n" % - (file_path, file_age - config.piggyback_max_cachefile_age)) + (file_path, file_age - piggyback_max_cachefile_age)) continue # Skip piggyback files that have not been updated in the last contact @@ -151,13 +150,13 @@ def store_piggyback_raw_data(sourcehost, piggybacked_raw_data): remove_source_status_file(sourcehost) -def cleanup_piggyback_files(): +def cleanup_piggyback_files(piggyback_max_cachefile_age): """This is a housekeeping job to clean up different old files from the piggyback directories. # Cleanup piggyback data of hosts that are not sending piggyback data anymore # a) hosts that have a file below piggyback_sources: - # -> check age of the file and remove it once it reached config.piggyback_max_cachefile_age + # -> check age of the file and remove it once it reached piggyback_max_cachefile_age # b) hosts that don't have a file below piggyback_sources (old version or removed by step "a)"): # -> remove all piggyback_raw_data files created by this source @@ -168,11 +167,11 @@ def cleanup_piggyback_files(): functions. Therefor all these functions needs to deal with suddenly vanishing or updated files/directories. """ - _cleanup_old_source_status_files() - _cleanup_old_piggybacked_files() + _cleanup_old_source_status_files(piggyback_max_cachefile_age) + _cleanup_old_piggybacked_files(piggyback_max_cachefile_age) -def _cleanup_old_source_status_files(): +def _cleanup_old_source_status_files(piggyback_max_cachefile_age): base_dir = os.path.join(cmk.paths.tmp_dir, "piggyback_sources") for entry in os.listdir(base_dir): if entry[0] == ".": @@ -185,11 +184,11 @@ def _cleanup_old_source_status_files(): except MKGeneralException, e: continue # File might've been deleted. That's ok. - if file_age > config.piggyback_max_cachefile_age: + if file_age > piggyback_max_cachefile_age: console.verbose("Removing outdated piggyback source status file %s\n" % file_path) _remove_piggyback_file(file_path) -def _cleanup_old_piggybacked_files(): +def _cleanup_old_piggybacked_files(piggyback_max_cachefile_age): """Remove piggyback data that is not needed anymore The monitoring (_get_piggyback_files()) is already skipping these files, @@ -214,7 +213,7 @@ def _cleanup_old_piggybacked_files(): file_path = os.path.join(backed_host_dir_path, source_host_name) - delete_reason = _shall_cleanup_piggyback_file(file_path, source_host_name, keep_sources) + delete_reason = _shall_cleanup_piggyback_file(piggyback_max_cachefile_age, file_path, source_host_name, keep_sources) if delete_reason: console.verbose("Removing outdated piggyback file (%s) %s\n" % (delete_reason, file_path)) _remove_piggyback_file(file_path) @@ -229,7 +228,7 @@ def _cleanup_old_piggybacked_files(): raise -def _shall_cleanup_piggyback_file(file_path, source_host_name, keep_sources): +def _shall_cleanup_piggyback_file(piggyback_max_cachefile_age, file_path, source_host_name, keep_sources): if source_host_name not in keep_sources: return "Source not sending piggyback data" @@ -239,8 +238,8 @@ def _shall_cleanup_piggyback_file(file_path, source_host_name, keep_sources): return None # File might've been deleted. That's ok. # Skip piggyback files that are outdated at all - if file_age > config.piggyback_max_cachefile_age: - return "%d seconds too old" % (file_age - config.piggyback_max_cachefile_age) + if file_age > piggyback_max_cachefile_age: + return "%d seconds too old" % (file_age - piggyback_max_cachefile_age) # Skip piggyback files that have not been updated in the last contact # with the source host that is currently being handled.