Merge pull request #1400 from YunoHost/lgtm-warnings

Lgtm warnings

data/hooks/diagnosis/12-dnsrecords.py

@@ -56,7 +56,6 @@ class DNSRecordsDiagnoser(Diagnoser):
     def check_domain(self, domain, is_main_domain):
 
         if is_special_use_tld(domain):
-            categories = []
             yield dict(
                 meta={"domain": domain},
                 data={},

src/yunohost/app.py

@@ -627,7 +627,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
             if upgrade_failed or broke_the_system:
 
                 # display this if there are remaining apps
-                if apps[number + 1 :]:
+                if apps[number + 1:]:
                     not_upgraded_apps = apps[number:]
                     logger.error(
                         m18n.n(
@@ -1033,7 +1033,6 @@ def app_remove(operation_logger, app, purge=False):
     remove_script = f"{tmp_workdir_for_app}/scripts/remove"
 
     env_dict = {}
-    app_id, app_instance_nb = _parse_app_instance_name(app)
     env_dict = _make_environment_for_app_script(app, workdir=tmp_workdir_for_app)
     env_dict["YNH_APP_PURGE"] = str(1 if purge else 0)
 
@@ -1627,9 +1626,12 @@ class AppConfigPanel(ConfigPanel):
                     error=message,
                 )
 
-    def _call_config_script(self, action, env={}):
+    def _call_config_script(self, action, env=None):
         from yunohost.hook import hook_exec
 
+        if env is None:
+            env = {}
+
         # Add default config script if needed
         config_script = os.path.join(
             APPS_SETTING_PATH, self.entity, "scripts", "config"

src/yunohost/certificate.py

@@ -70,28 +70,28 @@ PRODUCTION_CERTIFICATION_AUTHORITY = "https://acme-v02.api.letsencrypt.org"
 #
 
 
-def certificate_status(domain_list, full=False):
+def certificate_status(domains, full=False):
     """
     Print the status of certificate for given domains (all by default)
 
     Keyword argument:
-        domain_list -- Domains to be checked
+        domains     -- Domains to be checked
         full        -- Display more info about the certificates
     """
 
-    import yunohost.domain
+    from yunohost.domain import domain_list, _assert_domain_exists
 
     # If no domains given, consider all yunohost domains
-    if domain_list == []:
+    if domains == []:
-        domain_list = yunohost.domain.domain_list()["domains"]
+        domains = domain_list()["domains"]
     # Else, validate that yunohost knows the domains given
     else:
-        for domain in domain_list:
+        for domain in domains:
-            yunohost.domain._assert_domain_exists(domain)
+            _assert_domain_exists(domain)
 
     certificates = {}
 
-    for domain in domain_list:
+    for domain in domains:
         status = _get_status(domain)
 
         if not full:
@@ -239,28 +239,28 @@ def _certificate_install_selfsigned(domain_list, force=False):
 
 
 def _certificate_install_letsencrypt(
-    domain_list, force=False, no_checks=False, staging=False
+    domains, force=False, no_checks=False, staging=False
 ):
-    import yunohost.domain
+    from yunohost.domain import domain_list, _assert_domain_exists
 
     if not os.path.exists(ACCOUNT_KEY_FILE):
         _generate_account_key()
 
     # If no domains given, consider all yunohost domains with self-signed
     # certificates
-    if domain_list == []:
+    if domains == []:
-        for domain in yunohost.domain.domain_list()["domains"]:
+        for domain in domain_list()["domains"]:
 
             status = _get_status(domain)
             if status["CA_type"]["code"] != "self-signed":
                 continue
 
-            domain_list.append(domain)
+            domains.append(domain)
 
     # Else, validate that yunohost knows the domains given
     else:
-        for domain in domain_list:
+        for domain in domains:
-            yunohost.domain._assert_domain_exists(domain)
+            _assert_domain_exists(domain)
 
             # Is it self-signed?
             status = _get_status(domain)
@@ -275,7 +275,7 @@ def _certificate_install_letsencrypt(
         )
 
     # Actual install steps
-    for domain in domain_list:
+    for domain in domains:
 
         if not no_checks:
             try:
@@ -311,25 +311,25 @@ def _certificate_install_letsencrypt(
 
 
 def certificate_renew(
-    domain_list, force=False, no_checks=False, email=False, staging=False
+    domains, force=False, no_checks=False, email=False, staging=False
 ):
     """
     Renew Let's Encrypt certificate for given domains (all by default)
 
     Keyword argument:
-        domain_list -- Domains for which to renew the certificates
+        domains    -- Domains for which to renew the certificates
         force      -- Ignore the validity threshold (15 days)
         no-check   -- Disable some checks about the reachability of web server
                       before attempting the renewing
         email      -- Emails root if some renewing failed
     """
 
-    import yunohost.domain
+    from yunohost.domain import domain_list, _assert_domain_exists
 
     # If no domains given, consider all yunohost domains with Let's Encrypt
     # certificates
-    if domain_list == []:
+    if domains == []:
-        for domain in yunohost.domain.domain_list()["domains"]:
+        for domain in domain_list()["domains"]:
 
             # Does it have a Let's Encrypt cert?
             status = _get_status(domain)
@@ -347,17 +347,17 @@ def certificate_renew(
                 )
                 continue
 
-            domain_list.append(domain)
+            domains.append(domain)
 
-        if len(domain_list) == 0 and not email:
+        if len(domains) == 0 and not email:
             logger.info("No certificate needs to be renewed.")
 
     # Else, validate the domain list given
     else:
-        for domain in domain_list:
+        for domain in domains:
 
             # Is it in Yunohost domain list?
-            yunohost.domain._assert_domain_exists(domain)
+            _assert_domain_exists(domain)
 
             status = _get_status(domain)
 
@@ -385,7 +385,7 @@ def certificate_renew(
         )
 
     # Actual renew steps
-    for domain in domain_list:
+    for domain in domains:
 
         if not no_checks:
             try:

src/yunohost/data_migrations/0016_php70_to_php73_pools.py

@@ -38,11 +38,11 @@ class MyMigration(Migration):
         # Ignore the "www.conf" (default stuff, probably don't want to touch it ?)
         php70_pool_files = [f for f in php70_pool_files if f != "www.conf"]
 
-        for f in php70_pool_files:
+        for pf in php70_pool_files:
 
             # Copy the files to the php7.3 pool
-            src = "{}/{}".format(PHP70_POOLS, f)
+            src = "{}/{}".format(PHP70_POOLS, pf)
-            dest = "{}/{}".format(PHP73_POOLS, f)
+            dest = "{}/{}".format(PHP73_POOLS, pf)
             copy2(src, dest)
 
             # Replace the socket prefix if it's found
@@ -56,17 +56,17 @@ class MyMigration(Migration):
             c = "sed -i '1i {}' {}".format(MIGRATION_COMMENT, dest)
             os.system(c)
 
-            app_id = os.path.basename(f)[: -len(".conf")]
+            app_id = os.path.basename(pf)[: -len(".conf")]
             if _is_installed(app_id):
                 _patch_legacy_php_versions_in_settings(
                     "/etc/yunohost/apps/%s/" % app_id
                 )
 
             nginx_conf_files = glob.glob("/etc/nginx/conf.d/*.d/%s.conf" % app_id)
-            for f in nginx_conf_files:
+            for nf in nginx_conf_files:
                 # Replace the socket prefix if it's found
                 c = "sed -i -e 's@{}@{}@g' {}".format(
-                    PHP70_SOCKETS_PREFIX, PHP73_SOCKETS_PREFIX, f
+                    PHP70_SOCKETS_PREFIX, PHP73_SOCKETS_PREFIX, nf
                 )
                 os.system(c)
 

src/yunohost/data_migrations/0018_xtable_to_nftable.py

@@ -41,7 +41,8 @@ class MyMigration(Migration):
             )  # For some reason if we don't do this, iptables-legacy-save is empty ?
             self.runcmd("iptables-legacy-save > %s" % self.backup_rules_ipv4)
             assert (
-                open(self.backup_rules_ipv4).read().strip()
+                os.path.exists(self.backup_rules_ipv4) and
+                os.stat(self.backup_rules_ipv4).st_size > 0
             ), "Uhoh backup of legacy ipv4 rules is empty !?"
         if self.do_ipv6 and not os.path.exists(self.backup_rules_ipv6):
             self.runcmd(
@@ -49,7 +50,8 @@ class MyMigration(Migration):
             )  # For some reason if we don't do this, iptables-legacy-save is empty ?
             self.runcmd("ip6tables-legacy-save > %s" % self.backup_rules_ipv6)
             assert (
-                open(self.backup_rules_ipv6).read().strip()
+                os.path.exists(self.backup_rules_ipv6) and 
+                os.stat(self.backup_rules_ipv6).st_size > 0
             ), "Uhoh backup of legacy ipv6 rules is empty !?"
 
         # We inject the legacy rules (iptables-legacy) into the new iptable (just "iptables")

src/yunohost/dns.py

@@ -857,7 +857,6 @@ def domain_dns_push(operation_logger, domain, dry_run=False, force=False, purge=
             ignored = ""
 
         if action == "create":
-            old_content = record.get("old_content", "(None)")[:30]
             new_content = record.get("content", "(None)")[:30]
             return f"{name:>20} [{t:^5}] {new_content:^30}  {ignored}"
         elif action == "update":
@@ -867,7 +866,7 @@ def domain_dns_push(operation_logger, domain, dry_run=False, force=False, purge=
                 f"{name:>20} [{t:^5}] {old_content:^30} -> {new_content:^30}  {ignored}"
             )
         elif action == "unchanged":
-            old_content = new_content = record.get("content", "(None)")[:30]
+            old_content = record.get("content", "(None)")[:30]
             return f"{name:>20} [{t:^5}] {old_content:^30}"
         else:
             old_content = record.get("content", "(None)")[:30]

src/yunohost/domain.py

@@ -507,17 +507,17 @@ def _set_domain_settings(domain: str, settings: dict) -> None:
 
 
 def domain_cert_status(domain_list, full=False):
-    import yunohost.certificate
+    from yunohost.certificate import certificate_status
 
-    return yunohost.certificate.certificate_status(domain_list, full)
+    return certificate_status(domain_list, full)
 
 
 def domain_cert_install(
     domain_list, force=False, no_checks=False, self_signed=False, staging=False
 ):
-    import yunohost.certificate
+    from yunohost.certificate import certificate_install
 
-    return yunohost.certificate.certificate_install(
+    return certificate_install(
         domain_list, force, no_checks, self_signed, staging
     )
 
@@ -525,9 +525,9 @@ def domain_cert_install(
 def domain_cert_renew(
     domain_list, force=False, no_checks=False, email=False, staging=False
 ):
-    import yunohost.certificate
+    from yunohost.certificate import certificate_renew
 
-    return yunohost.certificate.certificate_renew(
+    return certificate_renew(
         domain_list, force, no_checks, email, staging
     )
 
@@ -537,12 +537,12 @@ def domain_dns_conf(domain):
 
 
 def domain_dns_suggest(domain):
-    import yunohost.dns
+    from yunohost.dns import domain_dns_suggest
 
-    return yunohost.dns.domain_dns_suggest(domain)
+    return domain_dns_suggest(domain)
 
 
 def domain_dns_push(domain, dry_run, force, purge):
-    import yunohost.dns
+    from yunohost.dns import domain_dns_push
 
-    return yunohost.dns.domain_dns_push(domain, dry_run, force, purge)
+    return domain_dns_push(domain, dry_run, force, purge)

src/yunohost/regenconf.py

@@ -48,7 +48,7 @@ logger = log.getActionLogger("yunohost.regenconf")
 @is_unit_operation([("names", "configuration")])
 def regen_conf(
     operation_logger,
-    names=[],
+    names=None,
     with_diff=False,
     force=False,
     dry_run=False,
@@ -66,6 +66,9 @@ def regen_conf(
 
     """
 
+    if names is None:
+        names = []
+
     result = {}
 
     # Return the list of pending conf

src/yunohost/service.py

@@ -587,7 +587,7 @@ def service_regen_conf(
         if name not in services.keys():
             raise YunohostValidationError("service_unknown", service=name)
 
-    if names is []:
+    if names == []:
         names = list(services.keys())
 
     logger.warning(m18n.n("service_regen_conf_is_deprecated"))

src/yunohost/tools.py

@@ -236,28 +236,24 @@ def tools_postinstall(
 
     # If this is a nohost.me/noho.st, actually check for availability
     if not ignore_dyndns and is_yunohost_dyndns_domain(domain):
-        # (Except if the user explicitly said he/she doesn't care about dyndns)
-        if ignore_dyndns:
-            dyndns = False
         # Check if the domain is available...
-        else:
+        try:
-            try:
+            available = _dyndns_available(domain)
-                available = _dyndns_available(domain)
+        # If an exception is thrown, most likely we don't have internet
-            # If an exception is thrown, most likely we don't have internet
+        # connectivity or something. Assume that this domain isn't manageable
-            # connectivity or something. Assume that this domain isn't manageable
+        # and inform the user that we could not contact the dyndns host server.
-            # and inform the user that we could not contact the dyndns host server.
+        except Exception:
-            except Exception:
+            logger.warning(
-                logger.warning(
+                m18n.n(
-                    m18n.n(
+                    "dyndns_provider_unreachable", provider="dyndns.yunohost.org"
-                        "dyndns_provider_unreachable", provider="dyndns.yunohost.org"
-                    )
                 )
+            )
 
-            if available:
+        if available:
-                dyndns = True
+            dyndns = True
-            # If not, abort the postinstall
+        # If not, abort the postinstall
-            else:
+        else:
-                raise YunohostValidationError("dyndns_unavailable", domain=domain)
+            raise YunohostValidationError("dyndns_unavailable", domain=domain)
     else:
         dyndns = False
 
@@ -1134,6 +1130,7 @@ class Migration:
     def description(self):
         return m18n.n("migration_description_%s" % self.id)
 
+    @staticmethod
     def ldap_migration(run):
         def func(self):
 

src/yunohost/user.py

@@ -1263,25 +1263,25 @@ def user_group_remove(groupname, usernames, force=False, sync_perm=True):
 
 
 def user_permission_list(short=False, full=False, apps=[]):
-    import yunohost.permission
+    from yunohost.permission import user_permission_list
 
-    return yunohost.permission.user_permission_list(
+    return user_permission_list(
         short, full, absolute_urls=True, apps=apps
     )
 
 
 def user_permission_update(permission, label=None, show_tile=None, sync_perm=True):
-    import yunohost.permission
+    from yunohost.permission import user_permission_update
 
-    return yunohost.permission.user_permission_update(
+    return user_permission_update(
         permission, label=label, show_tile=show_tile, sync_perm=sync_perm
     )
 
 
 def user_permission_add(permission, names, protected=None, force=False, sync_perm=True):
-    import yunohost.permission
+    from yunohost.permission import user_permission_update
 
-    return yunohost.permission.user_permission_update(
+    return user_permission_update(
         permission, add=names, protected=protected, force=force, sync_perm=sync_perm
     )
 
@@ -1289,23 +1289,23 @@ def user_permission_add(permission, names, protected=None, force=False, sync_per
 def user_permission_remove(
     permission, names, protected=None, force=False, sync_perm=True
 ):
-    import yunohost.permission
+    from yunohost.permission import user_permission_update
 
-    return yunohost.permission.user_permission_update(
+    return user_permission_update(
         permission, remove=names, protected=protected, force=force, sync_perm=sync_perm
     )
 
 
 def user_permission_reset(permission, sync_perm=True):
-    import yunohost.permission
+    from yunohost.permission import user_permission_reset
 
-    return yunohost.permission.user_permission_reset(permission, sync_perm=sync_perm)
+    return user_permission_reset(permission, sync_perm=sync_perm)
 
 
 def user_permission_info(permission):
-    import yunohost.permission
+    from yunohost.permission import user_permission_info
 
-    return yunohost.permission.user_permission_info(permission)
+    return user_permission_info(permission)
 
 
 #

src/yunohost/utils/config.py

@@ -52,7 +52,10 @@ CONFIG_PANEL_VERSION_SUPPORTED = 1.0
 # Those js-like evaluate functions are used to eval safely visible attributes
 # The goal is to evaluate in the same way than js simple-evaluate
 # https://github.com/shepherdwind/simple-evaluate
-def evaluate_simple_ast(node, context={}):
+def evaluate_simple_ast(node, context=None):
+    if context is None:
+        context = {}
+
     operators = {
         ast.Not: op.not_,
         ast.Mult: op.mul,