From f8ca33f7fbb98033e0e6a6436060d0f1d60b8698 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 17:31:30 +0100 Subject: [PATCH 1/9] Don't try to import. Use check_dependencies instead. --- synapse/config/metrics.py | 12 +++++------- synapse/config/repository.py | 24 ++++-------------------- synapse/python_dependencies.py | 5 ++++- 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 3698441963b1..2d9bf6054c66 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -13,11 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from ._base import Config, ConfigError +from synapse.python_dependencies import DependencyException, check_requirements -MISSING_SENTRY = """Missing sentry-sdk library. This is required to enable sentry - integration. - """ +from ._base import Config, ConfigError class MetricsConfig(Config): @@ -30,9 +28,9 @@ def read_config(self, config, **kwargs): self.sentry_enabled = "sentry" in config if self.sentry_enabled: try: - import sentry_sdk # noqa F401 - except ImportError: - raise ConfigError(MISSING_SENTRY) + check_requirements("sentry") + except DependencyException as e: + raise ConfigError(e.message) self.sentry_dsn = config["sentry"].get("dsn") if not self.sentry_dsn: diff --git a/synapse/config/repository.py b/synapse/config/repository.py index fdb1f246d086..732d742f97b0 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -16,6 +16,7 @@ import os from collections import namedtuple +from synapse.python_dependencies import DependencyException, check_requirements from synapse.util.module_loader import load_module from ._base import Config, ConfigError @@ -34,17 +35,6 @@ # method: %(method)s """ -MISSING_NETADDR = "Missing netaddr library. This is required for URL preview API." - -MISSING_LXML = """Missing lxml library. This is required for URL preview API. - - Install by running: - pip install lxml - - Requires libxslt1-dev system package. - """ - - ThumbnailRequirement = namedtuple( "ThumbnailRequirement", ["width", "height", "method", "media_type"] ) @@ -171,16 +161,10 @@ def read_config(self, config, **kwargs): self.url_preview_enabled = config.get("url_preview_enabled", False) if self.url_preview_enabled: try: - import lxml + check_requirements("url-preview-api") - lxml # To stop unused lint. - except ImportError: - raise ConfigError(MISSING_LXML) - - try: - from netaddr import IPSet - except ImportError: - raise ConfigError(MISSING_NETADDR) + except DependencyException as e: + raise ConfigError(e.message) if "url_preview_ip_range_blacklist" not in config: raise ConfigError( diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index ec0ac547c18b..9f2a6d1a2c9e 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -95,6 +95,7 @@ "sentry": ["sentry-sdk>=0.7.2"], "opentracing": ["jaeger-client>=4.0.0", "opentracing>=2.2.0"], "jwt": ["pyjwt>=1.6.4"], + "url-preview-api": ["netaddr", "lxml"], } ALL_OPTIONAL_REQUIREMENTS = set() @@ -147,7 +148,9 @@ def check_requirements(for_feature=None): ) except DistributionNotFound: deps_needed.append(dependency) - errors.append("Needed %s but it was not installed" % (dependency,)) + errors.append( + "Needed %s for %s but it was not installed" % (dependency, for_feature) + ) if not for_feature: # Check the optional dependencies are up to date. We allow them to not be From 63bc3867a39a8cde2074fe24a4e5e520f19a26b4 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 17:41:50 +0100 Subject: [PATCH 2/9] Nicer formatting --- synapse/python_dependencies.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 9f2a6d1a2c9e..fecabdb9f7da 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -149,7 +149,8 @@ def check_requirements(for_feature=None): except DistributionNotFound: deps_needed.append(dependency) errors.append( - "Needed %s for %s but it was not installed" % (dependency, for_feature) + "Needed %s for the '%s' feature but it was not installed" + % (dependency, for_feature) ) if not for_feature: From 6393052f5489d849daafdd0bf6db441929acf679 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 17:43:30 +0100 Subject: [PATCH 3/9] newsfile --- changelog.d/5989.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5989.misc diff --git a/changelog.d/5989.misc b/changelog.d/5989.misc new file mode 100644 index 000000000000..9f2525fd3ef4 --- /dev/null +++ b/changelog.d/5989.misc @@ -0,0 +1 @@ +Clean up dependency checking at setup. From eefb810e37f6193884b604b54e5e634e61169b9c Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 17:47:29 +0100 Subject: [PATCH 4/9] Missing import --- synapse/config/repository.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index 732d742f97b0..634744176f89 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -173,6 +173,8 @@ def read_config(self, config, **kwargs): "to work" ) + from netaddr import IPSet + self.url_preview_ip_range_blacklist = IPSet( config["url_preview_ip_range_blacklist"] ) From 56845ec370e48fdc3ec32f5d8e6a0443b14b7bc6 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 6 Sep 2019 10:39:11 +0100 Subject: [PATCH 5/9] lxml --- synapse/python_dependencies.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index fecabdb9f7da..0f24995d021c 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -95,7 +95,6 @@ "sentry": ["sentry-sdk>=0.7.2"], "opentracing": ["jaeger-client>=4.0.0", "opentracing>=2.2.0"], "jwt": ["pyjwt>=1.6.4"], - "url-preview-api": ["netaddr", "lxml"], } ALL_OPTIONAL_REQUIREMENTS = set() From 621aa5e473164774c613db7e54971ec44c118f33 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 6 Sep 2019 10:39:45 +0100 Subject: [PATCH 6/9] use correct feature --- synapse/config/repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index 634744176f89..44ddf938f2a7 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -161,7 +161,7 @@ def read_config(self, config, **kwargs): self.url_preview_enabled = config.get("url_preview_enabled", False) if self.url_preview_enabled: try: - check_requirements("url-preview-api") + check_requirements("url_preview") except DependencyException as e: raise ConfigError(e.message) From 8ed479c847cd1aae2e8f18e1fa16661698d2f057 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Tue, 10 Sep 2019 11:51:00 +0100 Subject: [PATCH 7/9] for_feature may be None. --- synapse/python_dependencies.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 0f24995d021c..ccdc7ddc097f 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -150,6 +150,8 @@ def check_requirements(for_feature=None): errors.append( "Needed %s for the '%s' feature but it was not installed" % (dependency, for_feature) + if for_feature + else "Needed %s but it was not installed" % (dependency,) ) if not for_feature: From df0b07c7ed0736e839f863bfcc53c51bcf44bb35 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Tue, 10 Sep 2019 11:54:33 +0100 Subject: [PATCH 8/9] comment on netaddr --- synapse/config/repository.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index 44ddf938f2a7..34f1a9a92dea 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -173,6 +173,7 @@ def read_config(self, config, **kwargs): "to work" ) + # netaddr is a dependency for url_preview from netaddr import IPSet self.url_preview_ip_range_blacklist = IPSet( From a72ccedc740c9787008d62654a8694177d5ce0b9 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Wed, 11 Sep 2019 13:29:09 +0100 Subject: [PATCH 9/9] Clearer conditional. --- synapse/python_dependencies.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index ccdc7ddc097f..07345e916af6 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -147,12 +147,13 @@ def check_requirements(for_feature=None): ) except DistributionNotFound: deps_needed.append(dependency) - errors.append( - "Needed %s for the '%s' feature but it was not installed" - % (dependency, for_feature) - if for_feature - else "Needed %s but it was not installed" % (dependency,) - ) + if for_feature: + errors.append( + "Needed %s for the '%s' feature but it was not installed" + % (dependency, for_feature) + ) + else: + errors.append("Needed %s but it was not installed" % (dependency,)) if not for_feature: # Check the optional dependencies are up to date. We allow them to not be