From 67f75805068086007f405de7df95f5e0697a9dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Thu, 20 Jul 2017 19:57:56 +0200 Subject: [PATCH] Only run connectivity checker if enabled Otherwise assume we are online but don't ping anything. See also #2011 --- docs/configuration/config_yaml.rst | 2 + src/octoprint/server/__init__.py | 7 ++- src/octoprint/server/api/settings.py | 2 + src/octoprint/settings.py | 1 + .../static/js/app/viewmodels/settings.js | 1 + .../templates/dialogs/settings/server.jinja2 | 29 ++------- .../settings/server/serverOnlineCheck.jinja2 | 18 ++++++ .../server/serverOnlineCheckEnabled.jinja2 | 7 +++ .../server/serverOnlineCheckHost.jinja2 | 6 ++ .../server/serverOnlineCheckInterval.jinja2 | 9 +++ .../server/serverOnlineCheckPort.jinja2 | 6 ++ src/octoprint/util/__init__.py | 62 +++++++++++++------ 12 files changed, 108 insertions(+), 42 deletions(-) create mode 100644 src/octoprint/templates/snippets/settings/server/serverOnlineCheck.jinja2 create mode 100644 src/octoprint/templates/snippets/settings/server/serverOnlineCheckEnabled.jinja2 create mode 100644 src/octoprint/templates/snippets/settings/server/serverOnlineCheckHost.jinja2 create mode 100644 src/octoprint/templates/snippets/settings/server/serverOnlineCheckInterval.jinja2 create mode 100644 src/octoprint/templates/snippets/settings/server/serverOnlineCheckPort.jinja2 diff --git a/docs/configuration/config_yaml.rst b/docs/configuration/config_yaml.rst index 9a68c58c..e934fb82 100644 --- a/docs/configuration/config_yaml.rst +++ b/docs/configuration/config_yaml.rst @@ -911,6 +911,8 @@ Use the following settings to configure the server: # Configuration of the regular online connectivity check onlineCheck: + # whether the online check is enabled, defaults to false due to valid privacy concerns + enabled: false # interval in which to check for online connectivity (in seconds) interval: 300 diff --git a/src/octoprint/server/__init__.py b/src/octoprint/server/__init__.py index 92a4be5f..d4ef1f79 100644 --- a/src/octoprint/server/__init__.py +++ b/src/octoprint/server/__init__.py @@ -231,6 +231,7 @@ class Server(object): preemptiveCache = PreemptiveCache(os.path.join(self._settings.getBaseFolder("data"), "preemptive_cache_config.yaml")) # start regular check if we are connected to the internet + connectivityEnabled = self._settings.getBoolean(["server", "onlineCheck", "enabled"]) connectivityInterval = self._settings.getInt(["server", "onlineCheck", "interval"]) connectivityHost = self._settings.get(["server", "onlineCheck", "host"]) connectivityPort = self._settings.getInt(["server", "onlineCheck", "port"]) @@ -241,17 +242,21 @@ class Server(object): connectivityChecker = octoprint.util.ConnectivityChecker(connectivityInterval, connectivityHost, port=connectivityPort, + enabled=connectivityEnabled, on_change=on_connectivity_change) def on_settings_update(*args, **kwargs): # make sure our connectivity checker runs with the latest settings + connectivityEnabled = self._settings.getBoolean(["server", "onlineCheck", "enabled"]) connectivityInterval = self._settings.getInt(["server", "onlineCheck", "interval"]) connectivityHost = self._settings.get(["server", "onlineCheck", "host"]) connectivityPort = self._settings.getInt(["server", "onlineCheck", "port"]) - if connectivityChecker.interval != connectivityInterval \ + if connectivityChecker.enabled != connectivityEnabled \ + or connectivityChecker.interval != connectivityInterval \ or connectivityChecker.host != connectivityHost \ or connectivityChecker.port != connectivityPort: + connectivityChecker.enabled = connectivityEnabled connectivityChecker.interval = connectivityInterval connectivityChecker.host = connectivityHost connectivityChecker.port = connectivityPort diff --git a/src/octoprint/server/api/settings.py b/src/octoprint/server/api/settings.py index 83639168..5112eca3 100644 --- a/src/octoprint/server/api/settings.py +++ b/src/octoprint/server/api/settings.py @@ -204,6 +204,7 @@ def getSettings(): "critical": s.getInt(["server", "diskspace", "critical"]) }, "onlineCheck": { + "enabled": s.getBoolean(["server", "onlineCheck", "enabled"]), "interval": int(s.getInt(["server", "onlineCheck", "interval"]) / 60), "host": s.get(["server", "onlineCheck", "host"]), "port": s.getInt(["server", "onlineCheck", "port"]) @@ -425,6 +426,7 @@ def _saveSettings(data): if "warning" in data["server"]["diskspace"]: s.setInt(["server", "diskspace", "warning"], data["server"]["diskspace"]["warning"]) if "critical" in data["server"]["diskspace"]: s.setInt(["server", "diskspace", "critical"], data["server"]["diskspace"]["critical"]) if "onlineCheck" in data["server"]: + if "enabled" in data["server"]["onlineCheck"]: s.setBoolean(["server", "onlineCheck", "enabled"], data["server"]["onlineCheck"]["enabled"]) if "interval" in data["server"]["onlineCheck"]: try: interval = int(data["server"]["onlineCheck"]["interval"]) diff --git a/src/octoprint/settings.py b/src/octoprint/settings.py index b688d92c..5e961521 100644 --- a/src/octoprint/settings.py +++ b/src/octoprint/settings.py @@ -149,6 +149,7 @@ default_settings = { "serverRestartCommand": None }, "onlineCheck": { + "enabled": None, "interval": 15 * 60, # 15 min "host": "8.8.8.8", "port": 53 diff --git a/src/octoprint/static/js/app/viewmodels/settings.js b/src/octoprint/static/js/app/viewmodels/settings.js index 3de17a36..1f5e6f32 100644 --- a/src/octoprint/static/js/app/viewmodels/settings.js +++ b/src/octoprint/static/js/app/viewmodels/settings.js @@ -213,6 +213,7 @@ $(function() { self.server_diskspace_warning_str = sizeObservable(self.server_diskspace_warning); self.server_diskspace_critical_str = sizeObservable(self.server_diskspace_critical); + self.server_onlineCheck_enabled = ko.observable(); self.server_onlineCheck_interval = ko.observable(); self.server_onlineCheck_host = ko.observable(); self.server_onlineCheck_port = ko.observable(); diff --git a/src/octoprint/templates/dialogs/settings/server.jinja2 b/src/octoprint/templates/dialogs/settings/server.jinja2 index c6b73b45..6fb59b47 100644 --- a/src/octoprint/templates/dialogs/settings/server.jinja2 +++ b/src/octoprint/templates/dialogs/settings/server.jinja2 @@ -7,29 +7,12 @@

{{ _('Connectivity check') }}

-

{{ _('You normally should not have to change any of the following settings.') }}

+

{% trans %} + If the connectivity check is enabled, OctoPrint will regularly check if it's connected to the internet. + This is useful to prevent resource intensive operations (such as checking for updates) if it's already + clear that they won't succeed anyhow. + {% endtrans %}

-
- -
- - - min - -
-
+ {% include "snippets/settings/server/serverOnlineCheck.jinja2" %} -
- -
- -
-
- -
- -
- -
-
diff --git a/src/octoprint/templates/snippets/settings/server/serverOnlineCheck.jinja2 b/src/octoprint/templates/snippets/settings/server/serverOnlineCheck.jinja2 new file mode 100644 index 00000000..abecffd3 --- /dev/null +++ b/src/octoprint/templates/snippets/settings/server/serverOnlineCheck.jinja2 @@ -0,0 +1,18 @@ +{% include "snippets/settings/server/serverOnlineCheckEnabled.jinja2" %} + +
+
+
+

{% trans %} + Define a check interval, a host and a port to check against. If you don't know what to set here, the + default values (using Google's DNS server) should work. If you have concerns about using + that, define the IP and port of a different online server that you trust and that has a high + availability. + {% endtrans %}

+
+
+ + {% include "snippets/settings/server/serverOnlineCheckInterval.jinja2" %} + {% include "snippets/settings/server/serverOnlineCheckHost.jinja2" %} + {% include "snippets/settings/server/serverOnlineCheckPort.jinja2" %} +
diff --git a/src/octoprint/templates/snippets/settings/server/serverOnlineCheckEnabled.jinja2 b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckEnabled.jinja2 new file mode 100644 index 00000000..11840a75 --- /dev/null +++ b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckEnabled.jinja2 @@ -0,0 +1,7 @@ +
+
+ +
+
diff --git a/src/octoprint/templates/snippets/settings/server/serverOnlineCheckHost.jinja2 b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckHost.jinja2 new file mode 100644 index 00000000..138a9430 --- /dev/null +++ b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckHost.jinja2 @@ -0,0 +1,6 @@ +
+ +
+ +
+
diff --git a/src/octoprint/templates/snippets/settings/server/serverOnlineCheckInterval.jinja2 b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckInterval.jinja2 new file mode 100644 index 00000000..d78b35d5 --- /dev/null +++ b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckInterval.jinja2 @@ -0,0 +1,9 @@ +
+ +
+ + + min + +
+
diff --git a/src/octoprint/templates/snippets/settings/server/serverOnlineCheckPort.jinja2 b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckPort.jinja2 new file mode 100644 index 00000000..541d8e7a --- /dev/null +++ b/src/octoprint/templates/snippets/settings/server/serverOnlineCheckPort.jinja2 @@ -0,0 +1,6 @@ +
+ +
+ +
+
diff --git a/src/octoprint/util/__init__.py b/src/octoprint/util/__init__.py index efd048bc..950946cf 100644 --- a/src/octoprint/util/__init__.py +++ b/src/octoprint/util/__init__.py @@ -1166,16 +1166,18 @@ class ConnectivityChecker(object): seconds and sets the ``online`` status accordingly. """ - def __init__(self, interval, host, port, on_change=None): + def __init__(self, interval, host, port, enabled=True, on_change=None): self._interval = interval self._host = host self._port = port + self._enabled = enabled self._on_change = on_change self._logger = logging.getLogger(__name__ + ".connectivity_checker") - self._last_check = None - self._online = False + # we initialize the online flag to True if we are not enabled (we don't know any better + # but these days it's probably a sane default) + self._online = not self._enabled self._check_worker = None self._check_mutex = threading.RLock() @@ -1188,12 +1190,6 @@ class ConnectivityChecker(object): with self._check_mutex: return self._online - @property - def last_check(self): - """Timestamp of last check.""" - with self._check_mutex: - return self._last_check - @property def host(self): """DNS host to query.""" @@ -1225,6 +1221,30 @@ class ConnectivityChecker(object): def interval(self, value): self._interval = value + @property + def enabled(self): + """Whether the check is enabled or not.""" + return self._enabled + + @enabled.setter + def enabled(self, value): + with self._check_mutex: + old_enabled = self._enabled + self._enabled = value + + if not self._enabled: + if self._check_worker is not None: + self._check_worker.cancel() + + old_value = self._online + self._online = True + + if old_value != self._online: + self._trigger_change(old_value, self._online) + + elif self._enabled and not old_enabled: + self._run() + def check_immediately(self): """Check immediately and return result.""" with self._check_mutex: @@ -1234,25 +1254,31 @@ class ConnectivityChecker(object): def _run(self): from octoprint.util import RepeatedTimer - if self._check_worker is not None: - raise RuntimeError("Connectivity manager check thread already active") + if not self._enabled: + return - self._check_worker = RepeatedTimer(self._interval, self._perform_check, run_first=True) + if self._check_worker is not None: + self._check_worker.cancel() + + self._check_worker = RepeatedTimer(self._interval, self._perform_check, + run_first=True) self._check_worker.start() def _perform_check(self): - import time + if not self._enabled: + return with self._check_mutex: self._logger.debug("Checking against {}:{} if we are online...".format(self._host, self._port)) old_value = self._online self._online = server_reachable(self._host, port=self._port) - self._last_check = time.time() if old_value != self._online: - self._logger.info("Connectivity changed from {} to {}".format("online" if old_value else "offline", - "online" if self._online else "offline")) - if callable(self._on_change): - self._on_change(old_value, self._online) + self._trigger_change(old_value, self._online) + def _trigger_change(self, old_value, new_value): + self._logger.info("Connectivity changed from {} to {}".format("online" if old_value else "offline", + "online" if new_value else "offline")) + if callable(self._on_change): + self._on_change(old_value, new_value)