From 30506fb80c8193b4913326144b1306e40e65bc9a Mon Sep 17 00:00:00 2001 From: Akiva Berger Date: Sun, 6 Aug 2023 12:51:16 +0300 Subject: [PATCH 1/5] fix(pytest): fix broken pytest after changing index error to log --- sefaria/model/tests/node_test.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sefaria/model/tests/node_test.py b/sefaria/model/tests/node_test.py index 451c35b5a2..22bed42c44 100644 --- a/sefaria/model/tests/node_test.py +++ b/sefaria/model/tests/node_test.py @@ -130,10 +130,12 @@ def test_terms_and_he(self): assert td == target - def test_bad_term(self): - with pytest.raises(IndexError): - j = JaggedArrayNode() - j.add_shared_term("BadTermName") + def test_bad_term(self, caplog): + j = JaggedArrayNode() + j.add_shared_term("BadTermName") + + assert f"Term BadTermName has no title_group" in caplog.text + #todo: why failing? @pytest.mark.xfail(reason="unknown") From 4ede00d32ff38da736340643ed96d4f0aa0942de Mon Sep 17 00:00:00 2001 From: Akiva Berger Date: Sun, 6 Aug 2023 14:51:56 +0300 Subject: [PATCH 2/5] fix(pytest): create graceful decorator for conditional exception --- .../configmap/local-settings-file.yaml | 7 +++++++ sefaria/local_settings_example.py | 8 +++++++- sefaria/model/schema.py | 5 ++++- sefaria/model/tests/node_test.py | 10 ++++------ sefaria/utils/util.py | 18 ++++++++++++++++++ 5 files changed, 40 insertions(+), 8 deletions(-) diff --git a/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml b/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml index b14c650fc9..a572ab9c01 100644 --- a/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml +++ b/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml @@ -15,6 +15,7 @@ data: structlog = None import sefaria.system.logging as sefaria_logging import os + import sys import re import json @@ -328,3 +329,9 @@ data: wrapper_class=structlog.stdlib.BoundLogger, cache_logger_on_first_use=True, ) + + + # Fail gracefully when a text or ref cannot be properly loaded, this should be set to True on production + FAIL_TEXT_GRACEFULLY = False + if "pytest" in sys.modules: + FAIL_TEXT_GRACEFULLY = False \ No newline at end of file diff --git a/sefaria/local_settings_example.py b/sefaria/local_settings_example.py index 48e9bd0176..db4bca356b 100644 --- a/sefaria/local_settings_example.py +++ b/sefaria/local_settings_example.py @@ -1,6 +1,7 @@ # An example of settings needed in a local_settings.py file. # copy this file to sefaria/local_settings.py and provide local info to run. from datetime import timedelta +import sys import structlog import sefaria.system.logging as sefaria_logging import os @@ -308,4 +309,9 @@ ) SENTRY_DSN = None -CLIENT_SENTRY_DSN = None \ No newline at end of file +CLIENT_SENTRY_DSN = None + +# Fail gracefully when a text or ref cannot be properly loaded, this should be set to True on production +FAIL_TEXT_GRACEFULLY = False +if "pytest" in sys.modules: + FAIL_TEXT_GRACEFULLY = False \ No newline at end of file diff --git a/sefaria/model/schema.py b/sefaria/model/schema.py index 1d22532b83..0436dc7831 100644 --- a/sefaria/model/schema.py +++ b/sefaria/model/schema.py @@ -5,6 +5,8 @@ import structlog from functools import reduce + +from sefaria.utils.util import conditional_graceful_exception logger = structlog.get_logger(__name__) try: @@ -210,6 +212,7 @@ def _load_title_group(self): self._process_terms() + @conditional_graceful_exception(logger=logger, return_value=None) def _process_terms(self): # To be called after raw data load from sefaria.model import library @@ -219,7 +222,7 @@ def _process_terms(self): try: self.title_group = term.title_group except AttributeError: - logger.error(f"Term {self.sharedTitle} has no title_group") + raise IndexError("Failed to load term named {}.".format(self.sharedTitle)) def add_shared_term(self, term): self.sharedTitle = term diff --git a/sefaria/model/tests/node_test.py b/sefaria/model/tests/node_test.py index 22bed42c44..451c35b5a2 100644 --- a/sefaria/model/tests/node_test.py +++ b/sefaria/model/tests/node_test.py @@ -130,12 +130,10 @@ def test_terms_and_he(self): assert td == target - def test_bad_term(self, caplog): - j = JaggedArrayNode() - j.add_shared_term("BadTermName") - - assert f"Term BadTermName has no title_group" in caplog.text - + def test_bad_term(self): + with pytest.raises(IndexError): + j = JaggedArrayNode() + j.add_shared_term("BadTermName") #todo: why failing? @pytest.mark.xfail(reason="unknown") diff --git a/sefaria/utils/util.py b/sefaria/utils/util.py index 28cd48ada1..36180d201a 100644 --- a/sefaria/utils/util.py +++ b/sefaria/utils/util.py @@ -8,6 +8,7 @@ from functools import wraps from itertools import zip_longest from sefaria.constants.model import ALLOWED_TAGS_IN_ABSTRACT_TEXT_RECORD +from sefaria.settings import FAIL_TEXT_GRACEFULLY """ Time utils @@ -611,3 +612,20 @@ def decorated_function(*args, **kwargs): return return_value return decorated_function return argumented_decorator + + +def conditional_graceful_exception(logger=None, logLevel="exception", return_value=[], exception_type=Exception): + def argumented_decorator(func): + @wraps(func) + def decorated_function(*args, **kwargs): + try: + return func(*args, **kwargs) + except exception_type as e: + if FAIL_TEXT_GRACEFULLY: + if logger: + logger.exception(str(e)) if logLevel == "exception" else logger.warning(str(e)) + else: + raise e + return return_value + return decorated_function + return argumented_decorator \ No newline at end of file From f327b597393109314cfc7352ce1dc6b543f75357 Mon Sep 17 00:00:00 2001 From: Akiva Berger Date: Sun, 6 Aug 2023 15:00:19 +0300 Subject: [PATCH 3/5] fix(pytest): rename var --- .../templates/configmap/local-settings-file.yaml | 4 ++-- sefaria/local_settings_example.py | 4 ++-- sefaria/utils/util.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml b/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml index a572ab9c01..9d6f2de2ac 100644 --- a/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml +++ b/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml @@ -332,6 +332,6 @@ data: # Fail gracefully when a text or ref cannot be properly loaded, this should be set to True on production - FAIL_TEXT_GRACEFULLY = False + FAIL_GRACEFULLY = False if "pytest" in sys.modules: - FAIL_TEXT_GRACEFULLY = False \ No newline at end of file + FAIL_GRACEFULLY = False \ No newline at end of file diff --git a/sefaria/local_settings_example.py b/sefaria/local_settings_example.py index db4bca356b..740809a3ab 100644 --- a/sefaria/local_settings_example.py +++ b/sefaria/local_settings_example.py @@ -312,6 +312,6 @@ CLIENT_SENTRY_DSN = None # Fail gracefully when a text or ref cannot be properly loaded, this should be set to True on production -FAIL_TEXT_GRACEFULLY = False +FAIL_GRACEFULLY = False if "pytest" in sys.modules: - FAIL_TEXT_GRACEFULLY = False \ No newline at end of file + FAIL_GRACEFULLY = False \ No newline at end of file diff --git a/sefaria/utils/util.py b/sefaria/utils/util.py index 36180d201a..8c9413268c 100644 --- a/sefaria/utils/util.py +++ b/sefaria/utils/util.py @@ -8,7 +8,7 @@ from functools import wraps from itertools import zip_longest from sefaria.constants.model import ALLOWED_TAGS_IN_ABSTRACT_TEXT_RECORD -from sefaria.settings import FAIL_TEXT_GRACEFULLY +from sefaria.settings import FAIL_GRACEFULLY """ Time utils @@ -621,7 +621,7 @@ def decorated_function(*args, **kwargs): try: return func(*args, **kwargs) except exception_type as e: - if FAIL_TEXT_GRACEFULLY: + if FAIL_GRACEFULLY: if logger: logger.exception(str(e)) if logLevel == "exception" else logger.warning(str(e)) else: From 240b038f66d87c2b0d7b06831a169a0c45c4612d Mon Sep 17 00:00:00 2001 From: Akiva Berger Date: Mon, 7 Aug 2023 09:24:38 +0300 Subject: [PATCH 4/5] fix(pytest): correct var value --- .../templates/configmap/local-settings-file.yaml | 5 +++-- sefaria/local_settings_example.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml b/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml index 9d6f2de2ac..a2300a2e18 100644 --- a/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml +++ b/helm-chart/sefaria-project/templates/configmap/local-settings-file.yaml @@ -331,7 +331,8 @@ data: ) - # Fail gracefully when a text or ref cannot be properly loaded, this should be set to True on production - FAIL_GRACEFULLY = False + # Fail gracefully when decorator conditional_graceful_exception on function. This should be set to True on production + # Example: If a text or ref cannot be properly loaded, fail gracefully and let the server continue to run + FAIL_GRACEFULLY = True if "pytest" in sys.modules: FAIL_GRACEFULLY = False \ No newline at end of file diff --git a/sefaria/local_settings_example.py b/sefaria/local_settings_example.py index 740809a3ab..f6b1fbf201 100644 --- a/sefaria/local_settings_example.py +++ b/sefaria/local_settings_example.py @@ -311,7 +311,8 @@ SENTRY_DSN = None CLIENT_SENTRY_DSN = None -# Fail gracefully when a text or ref cannot be properly loaded, this should be set to True on production +# Fail gracefully when decorator conditional_graceful_exception on function. This should be set to True on production +# Example: If a text or ref cannot be properly loaded, fail gracefully and let the server continue to run FAIL_GRACEFULLY = False if "pytest" in sys.modules: FAIL_GRACEFULLY = False \ No newline at end of file From 7b2f2c5d8e6e2de2c0cfc0986ec7c5941e185fea Mon Sep 17 00:00:00 2001 From: Akiva Berger Date: Mon, 7 Aug 2023 13:51:01 +0300 Subject: [PATCH 5/5] fix(pytest): documentation on decorator --- sefaria/model/schema.py | 6 ++++-- sefaria/system/decorators.py | 30 ++++++++++++++++++++++++++++++ sefaria/utils/util.py | 18 ------------------ 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/sefaria/model/schema.py b/sefaria/model/schema.py index 0436dc7831..305dda01b8 100644 --- a/sefaria/model/schema.py +++ b/sefaria/model/schema.py @@ -6,7 +6,9 @@ import structlog from functools import reduce -from sefaria.utils.util import conditional_graceful_exception +from sefaria.system.decorators import conditional_graceful_exception + + logger = structlog.get_logger(__name__) try: @@ -212,7 +214,7 @@ def _load_title_group(self): self._process_terms() - @conditional_graceful_exception(logger=logger, return_value=None) + @conditional_graceful_exception() def _process_terms(self): # To be called after raw data load from sefaria.model import library diff --git a/sefaria/system/decorators.py b/sefaria/system/decorators.py index 14e949b67d..94db751aea 100644 --- a/sefaria/system/decorators.py +++ b/sefaria/system/decorators.py @@ -1,4 +1,5 @@ from functools import wraps, partial +from typing import Any from django.http import HttpResponse, Http404 from django.template import RequestContext @@ -13,6 +14,8 @@ import structlog logger = structlog.get_logger(__name__) +from sefaria.settings import FAIL_GRACEFULLY + # TODO: we really need to fix the way we are using json responses. Django 1.7 introduced a baked in JsonResponse. def json_response_decorator(func): @@ -95,6 +98,33 @@ def wrapper(request, *args, **kwargs): return wrapper +def conditional_graceful_exception(logLevel: str = "exception", exception_type: Exception = Exception) -> Any: + """ + Decorator that catches exceptions and logs them if FAIL_GRACEFULLY is True. + For instance, when loading the server on prod, we want to fail gracefully if a text or ref cannot be properly loaded. + However, when running text creation functions, we want to fail loudly so that we can fix the problem. + + :param logLevel: "exception" or "warning" + :param return_value: function return value if exception is caught + :param exception_type: type of exception to catch + :return: return_value no error, if FAIL_GRACEFULLY is True log the error, otherwise raise exception + + """ + def argumented_decorator(func): + @wraps(func) + def decorated_function(*args, **kwargs): + try: + return func(*args, **kwargs) + except exception_type as e: + if FAIL_GRACEFULLY: + if logger: + logger.exception(str(e)) if logLevel == "exception" else logger.warning(str(e)) + else: + raise e + return decorated_function + return argumented_decorator + + class memoized(object): """Decorator. Caches a function's return value each time it is called. If called later with the same arguments, the cached value is returned diff --git a/sefaria/utils/util.py b/sefaria/utils/util.py index 8c9413268c..28cd48ada1 100644 --- a/sefaria/utils/util.py +++ b/sefaria/utils/util.py @@ -8,7 +8,6 @@ from functools import wraps from itertools import zip_longest from sefaria.constants.model import ALLOWED_TAGS_IN_ABSTRACT_TEXT_RECORD -from sefaria.settings import FAIL_GRACEFULLY """ Time utils @@ -612,20 +611,3 @@ def decorated_function(*args, **kwargs): return return_value return decorated_function return argumented_decorator - - -def conditional_graceful_exception(logger=None, logLevel="exception", return_value=[], exception_type=Exception): - def argumented_decorator(func): - @wraps(func) - def decorated_function(*args, **kwargs): - try: - return func(*args, **kwargs) - except exception_type as e: - if FAIL_GRACEFULLY: - if logger: - logger.exception(str(e)) if logLevel == "exception" else logger.warning(str(e)) - else: - raise e - return return_value - return decorated_function - return argumented_decorator \ No newline at end of file