This 'args' things sounds like a big YAGNI after all

This commit is contained in:
Alexandre Aubin 2019-07-20 19:02:11 +02:00
parent 24f9d475b8
commit d2bbb5a2b3
6 changed files with 54 additions and 85 deletions

View file

@ -1898,9 +1898,6 @@ diagnosis:
--force: --force:
help: Ignore the cached report even if it is still 'fresh' help: Ignore the cached report even if it is still 'fresh'
action: store_true action: store_true
-a:
help: Serialized arguments for diagnosis scripts (e.g. "domain=domain.tld")
full: --args
ignore: ignore:
action_help: Configure some diagnosis results to be ignored action_help: Configure some diagnosis results to be ignored

View file

@ -15,18 +15,11 @@ class IPDiagnoser(Diagnoser):
id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
cache_duration = 60 cache_duration = 60
def validate_args(self, args):
if "version" not in args.keys():
return {"versions": [4, 6]}
else:
assert str(args["version"]) in ["4", "6"], "Invalid version, should be 4 or 6."
return {"versions": [int(args["version"])]}
def run(self): def run(self):
versions = self.args["versions"] #
# IPv4 Diagnosis
if 4 in versions: #
# If we can't ping, there's not much else we can do # If we can't ping, there's not much else we can do
if not self.can_ping_outside(4): if not self.can_ping_outside(4):
@ -67,7 +60,9 @@ class IPDiagnoser(Diagnoser):
summary=("diagnosis_ip_connected_ipv4", {}) if ipv4 summary=("diagnosis_ip_connected_ipv4", {}) if ipv4
else ("diagnosis_ip_no_ipv4", {})) else ("diagnosis_ip_no_ipv4", {}))
if 6 in versions: #
# IPv6 Diagnosis
#
if not self.can_ping_outside(4): if not self.can_ping_outside(4):
ipv6 = None ipv6 = None

View file

@ -14,14 +14,6 @@ class DNSRecordsDiagnoser(Diagnoser):
id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
cache_duration = 3600 * 24 cache_duration = 3600 * 24
def validate_args(self, args):
all_domains = domain_list()["domains"]
if "domain" not in args.keys():
return {"domains": all_domains}
else:
assert args["domain"] in all_domains, "Unknown domain"
return {"domains": [args["domain"]]}
def run(self): def run(self):
resolvers = read_file("/etc/resolv.dnsmasq.conf").split("\n") resolvers = read_file("/etc/resolv.dnsmasq.conf").split("\n")
@ -32,7 +24,8 @@ class DNSRecordsDiagnoser(Diagnoser):
self.resolver = ipv4_resolvers[0] self.resolver = ipv4_resolvers[0]
main_domain = _get_maindomain() main_domain = _get_maindomain()
for domain in self.args["domains"]: all_domains = domain_list()["domains"]
for domain in all_domains:
self.logger_debug("Diagnosing DNS conf for %s" % domain) self.logger_debug("Diagnosing DNS conf for %s" % domain)
for report in self.check_domain(domain, domain == main_domain): for report in self.check_domain(domain, domain == main_domain):
yield report yield report

View file

@ -21,10 +21,6 @@ class ServicesDiagnoser(Diagnoser):
id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
cache_duration = 300 cache_duration = 300
def validate_args(self, args):
# TODO / FIXME Ugh do we really need this arg system
return {}
def run(self): def run(self):
all_result = service_status() all_result = service_status()

View file

@ -9,10 +9,6 @@ class DiskUsageDiagnoser(Diagnoser):
id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1] id_ = os.path.splitext(os.path.basename(__file__))[0].split("-")[1]
cache_duration = 3600 * 24 cache_duration = 3600 * 24
def validate_args(self, args):
# TODO / FIXME Ugh do we really need this arg system
return {}
def run(self): def run(self):
disk_partitions = psutil.disk_partitions() disk_partitions = psutil.disk_partitions()

View file

@ -84,7 +84,7 @@ def diagnosis_show(categories=[], issues=False, full=False):
return {"reports": all_reports} return {"reports": all_reports}
def diagnosis_run(categories=[], force=False, args=None): def diagnosis_run(categories=[], force=False):
# Get all the categories # Get all the categories
all_categories = _list_diagnosis_categories() all_categories = _list_diagnosis_categories()
@ -98,13 +98,6 @@ def diagnosis_run(categories=[], force=False, args=None):
if unknown_categories: if unknown_categories:
raise YunohostError('unknown_categories', categories=", ".join(unknown_categories)) raise YunohostError('unknown_categories', categories=", ".join(unknown_categories))
# Transform "arg1=val1&arg2=val2" to { "arg1": "val1", "arg2": "val2" }
if args is not None:
args = {arg.split("=")[0]: arg.split("=")[1] for arg in args.split("&")}
else:
args = {}
args["force"] = force
issues = [] issues = []
# Call the hook ... # Call the hook ...
diagnosed_categories = [] diagnosed_categories = []
@ -113,7 +106,7 @@ def diagnosis_run(categories=[], force=False, args=None):
path = [p for n, p in all_categories if n == category][0] path = [p for n, p in all_categories if n == category][0]
try: try:
code, report = hook_exec(path, args=args, env=None) code, report = hook_exec(path, args={"force": force}, env=None)
except Exception as e: except Exception as e:
logger.error("Diagnosis failed for category '%s' : %s" % (category, str(e)), exc_info=True) # FIXME : i18n logger.error("Diagnosis failed for category '%s' : %s" % (category, str(e)), exc_info=True) # FIXME : i18n
else: else:
@ -143,7 +136,6 @@ class Diagnoser():
self.logger_debug, self.logger_warning, self.logger_info = loggers self.logger_debug, self.logger_warning, self.logger_info = loggers
self.env = env self.env = env
self.args = args or {} self.args = args or {}
self.args.update(self.validate_args(self.args))
self.cache_file = Diagnoser.cache_file(self.id_) self.cache_file = Diagnoser.cache_file(self.id_)
descr_key = "diagnosis_description_" + self.id_ descr_key = "diagnosis_description_" + self.id_