From 235ac8b2a619ecfa61873eae7a608d328958c17a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 14 May 2024 20:46:12 -0300 Subject: [PATCH 1/6] refactor: idnits2 mgmt cmds -> tasks (#7421) * feat: tasks for generate_idnits2_rfc* mgmt cmds * chore: create periodic tasks * chore: remove mgmt cmds from bin/hourly * test: test new tasks * chore: remove now-unused scripts * refactor: unitize Idnits2SupportTests --- bin/hourly | 3 -- .../commands/generate_idnits2_rfc_status.py | 23 ----------- .../generate_idnits2_rfcs_obsoleted.py | 23 ----------- ietf/doc/tasks.py | 24 +++++++++++ ietf/doc/tests.py | 30 ++++++++++---- ietf/doc/tests_tasks.py | 40 ++++++++++++++++--- .../management/commands/periodic_tasks.py | 20 ++++++++++ 7 files changed, 101 insertions(+), 62 deletions(-) delete mode 100644 ietf/doc/management/commands/generate_idnits2_rfc_status.py delete mode 100644 ietf/doc/management/commands/generate_idnits2_rfcs_obsoleted.py diff --git a/bin/hourly b/bin/hourly index 9478bec11..81638fa54 100755 --- a/bin/hourly +++ b/bin/hourly @@ -24,9 +24,6 @@ ID=/a/ietfdata/doc/draft/repository DERIVED=/a/ietfdata/derived DOWNLOAD=/a/www/www6s/download -$DTDIR/ietf/manage.py generate_idnits2_rfc_status -$DTDIR/ietf/manage.py generate_idnits2_rfcs_obsoleted - CHARTER=/a/www/ietf-ftp/charter wget -q https://datatracker.ietf.org/wg/1wg-charters-by-acronym.txt -O $CHARTER/1wg-charters-by-acronym.txt wget -q https://datatracker.ietf.org/wg/1wg-charters.txt -O $CHARTER/1wg-charters.txt diff --git a/ietf/doc/management/commands/generate_idnits2_rfc_status.py b/ietf/doc/management/commands/generate_idnits2_rfc_status.py deleted file mode 100644 index 45be18801..000000000 --- a/ietf/doc/management/commands/generate_idnits2_rfc_status.py +++ /dev/null @@ -1,23 +0,0 @@ -# Copyright The IETF Trust 2021 All Rights Reserved - -import os - -from django.conf import settings -from django.core.management.base import BaseCommand - -from ietf.doc.utils import generate_idnits2_rfc_status -from ietf.utils.log import log - -class Command(BaseCommand): - help = ('Generate the rfc_status blob used by idnits2') - - def handle(self, *args, **options): - filename=os.path.join(settings.DERIVED_DIR,'idnits2-rfc-status') - blob = generate_idnits2_rfc_status() - try: - bytes = blob.encode('utf-8') - with open(filename,'wb') as f: - f.write(bytes) - except Exception as e: - log('failed to write idnits2-rfc-status: '+str(e)) - raise e diff --git a/ietf/doc/management/commands/generate_idnits2_rfcs_obsoleted.py b/ietf/doc/management/commands/generate_idnits2_rfcs_obsoleted.py deleted file mode 100644 index 8bd122e87..000000000 --- a/ietf/doc/management/commands/generate_idnits2_rfcs_obsoleted.py +++ /dev/null @@ -1,23 +0,0 @@ -# Copyright The IETF Trust 2021 All Rights Reserved - -import os - -from django.conf import settings -from django.core.management.base import BaseCommand - -from ietf.doc.utils import generate_idnits2_rfcs_obsoleted -from ietf.utils.log import log - -class Command(BaseCommand): - help = ('Generate the rfcs-obsoleted file used by idnits2') - - def handle(self, *args, **options): - filename=os.path.join(settings.DERIVED_DIR,'idnits2-rfcs-obsoleted') - blob = generate_idnits2_rfcs_obsoleted() - try: - bytes = blob.encode('utf-8') - with open(filename,'wb') as f: - f.write(bytes) - except Exception as e: - log('failed to write idnits2-rfcs-obsoleted: '+str(e)) - raise e diff --git a/ietf/doc/tasks.py b/ietf/doc/tasks.py index a2e83e9e2..b189a6827 100644 --- a/ietf/doc/tasks.py +++ b/ietf/doc/tasks.py @@ -6,6 +6,9 @@ import datetime import debug # pyflakes:ignore from celery import shared_task +from pathlib import Path + +from django.conf import settings from ietf.utils import log from ietf.utils.timezone import datetime_today @@ -21,6 +24,7 @@ from .expire import ( send_expire_warning_for_draft, ) from .models import Document +from .utils import generate_idnits2_rfc_status, generate_idnits2_rfcs_obsoleted @shared_task @@ -54,3 +58,23 @@ def expire_ids_task(): def notify_expirations_task(notify_days=14): for doc in get_soon_to_expire_drafts(notify_days): send_expire_warning_for_draft(doc) + + +@shared_task +def generate_idnits2_rfc_status_task(): + outpath = Path(settings.DERIVED_DIR) / "idnits2-rfc-status" + blob = generate_idnits2_rfc_status() + try: + outpath.write_text(blob, encoding="utf8") + except Exception as e: + log.log(f"failed to write idnits2-rfc-status: {e}") + + +@shared_task +def generate_idnits2_rfcs_obsoleted_task(): + outpath = Path(settings.DERIVED_DIR) / "idnits2-rfcs-obsoleted" + blob = generate_idnits2_rfcs_obsoleted() + try: + outpath.write_text(blob, encoding="utf8") + except Exception as e: + log.log(f"failed to write idnits2-rfcs-obsoleted: {e}") diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 4f5492e6c..e6a50937a 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -20,7 +20,6 @@ from tempfile import NamedTemporaryFile from collections import defaultdict from zoneinfo import ZoneInfo -from django.core.management import call_command from django.urls import reverse as urlreverse from django.conf import settings from django.forms import Form @@ -45,7 +44,14 @@ from ietf.doc.factories import ( DocumentFactory, DocEventFactory, CharterFactor StatusChangeFactory, DocExtResourceFactory, RgDraftFactory, BcpFactory) from ietf.doc.forms import NotifyForm from ietf.doc.fields import SearchableDocumentsField -from ietf.doc.utils import create_ballot_if_not_open, investigate_fragment, uppercase_std_abbreviated_name, DraftAliasGenerator +from ietf.doc.utils import ( + create_ballot_if_not_open, + investigate_fragment, + uppercase_std_abbreviated_name, + DraftAliasGenerator, + generate_idnits2_rfc_status, + generate_idnits2_rfcs_obsoleted, +) from ietf.group.models import Group, Role from ietf.group.factories import GroupFactory, RoleFactory from ietf.ipr.factories import HolderIprDisclosureFactory @@ -2831,32 +2837,40 @@ class MaterialsTests(TestCase): class Idnits2SupportTests(TestCase): settings_temp_path_overrides = TestCase.settings_temp_path_overrides + ['DERIVED_DIR'] - def test_obsoleted(self): + def test_generate_idnits2_rfcs_obsoleted(self): rfc = WgRfcFactory(rfc_number=1001) WgRfcFactory(rfc_number=1003,relations=[('obs',rfc)]) rfc = WgRfcFactory(rfc_number=1005) WgRfcFactory(rfc_number=1007,relations=[('obs',rfc)]) + blob = generate_idnits2_rfcs_obsoleted() + self.assertEqual(blob, b'1001 1003\n1005 1007\n'.decode("utf8")) + def test_obsoleted(self): url = urlreverse('ietf.doc.views_doc.idnits2_rfcs_obsoleted') r = self.client.get(url) self.assertEqual(r.status_code, 404) - call_command('generate_idnits2_rfcs_obsoleted') + # value written is arbitrary, expect it to be passed through + (Path(settings.DERIVED_DIR) / "idnits2-rfcs-obsoleted").write_bytes(b'1001 1003\n1005 1007\n') url = urlreverse('ietf.doc.views_doc.idnits2_rfcs_obsoleted') r = self.client.get(url) self.assertEqual(r.status_code, 200) self.assertEqual(r.content, b'1001 1003\n1005 1007\n') - def test_rfc_status(self): + def test_generate_idnits2_rfc_status(self): for slug in ('bcp', 'ds', 'exp', 'hist', 'inf', 'std', 'ps', 'unkn'): WgRfcFactory(std_level_id=slug) + blob = generate_idnits2_rfc_status().replace("\n", "") + self.assertEqual(blob[6312-1], "O") + + def test_rfc_status(self): url = urlreverse('ietf.doc.views_doc.idnits2_rfc_status') r = self.client.get(url) self.assertEqual(r.status_code,404) - call_command('generate_idnits2_rfc_status') + # value written is arbitrary, expect it to be passed through + (Path(settings.DERIVED_DIR) / "idnits2-rfc-status").write_bytes(b'1001 1003\n1005 1007\n') r = self.client.get(url) self.assertEqual(r.status_code,200) - blob = unicontent(r).replace('\n','') - self.assertEqual(blob[6312-1],'O') + self.assertEqual(r.content, b'1001 1003\n1005 1007\n') def test_idnits2_state(self): rfc = WgRfcFactory() diff --git a/ietf/doc/tests_tasks.py b/ietf/doc/tests_tasks.py index 931ed438d..8915a6c5a 100644 --- a/ietf/doc/tests_tasks.py +++ b/ietf/doc/tests_tasks.py @@ -1,15 +1,24 @@ # Copyright The IETF Trust 2024, All Rights Reserved import mock +from pathlib import Path + +from django.conf import settings + from ietf.utils.test_utils import TestCase from ietf.utils.timezone import datetime_today from .factories import DocumentFactory from .models import Document -from .tasks import expire_ids_task, notify_expirations_task - +from .tasks import ( + expire_ids_task, + generate_idnits2_rfcs_obsoleted_task, + generate_idnits2_rfc_status_task, + notify_expirations_task, +) class TaskTests(TestCase): + settings_temp_path_overrides = TestCase.settings_temp_path_overrides + ["DERIVED_DIR"] @mock.patch("ietf.doc.tasks.in_draft_expire_freeze") @mock.patch("ietf.doc.tasks.get_expired_drafts") @@ -35,10 +44,10 @@ class TaskTests(TestCase): Document.objects.filter(pk=doc.pk), Document.objects.filter(pk=other_doc.pk), ] - + # call task expire_ids_task() - + # check results self.assertTrue(in_draft_expire_freeze_mock.called) self.assertEqual(expirable_drafts_mock.call_count, 2) @@ -50,7 +59,7 @@ class TaskTests(TestCase): # test that an exception is raised in_draft_expire_freeze_mock.side_effect = RuntimeError - with self.assertRaises(RuntimeError):( + with self.assertRaises(RuntimeError): ( expire_ids_task()) @mock.patch("ietf.doc.tasks.send_expire_warning_for_draft") @@ -61,3 +70,24 @@ class TaskTests(TestCase): notify_expirations_task() self.assertEqual(send_warning_mock.call_count, 1) self.assertEqual(send_warning_mock.call_args[0], ("sentinel",)) + + @mock.patch("ietf.doc.tasks.generate_idnits2_rfc_status") + def test_generate_idnits2_rfc_status_task(self, mock_generate): + mock_generate.return_value = "dåtå" + generate_idnits2_rfc_status_task() + self.assertEqual(mock_generate.call_count, 1) + self.assertEqual( + "dåtå".encode("utf8"), + (Path(settings.DERIVED_DIR) / "idnits2-rfc-status").read_bytes(), + ) + + @mock.patch("ietf.doc.tasks.generate_idnits2_rfcs_obsoleted") + def test_generate_idnits2_rfcs_obsoleted_task(self, mock_generate): + mock_generate.return_value = "dåtå" + generate_idnits2_rfcs_obsoleted_task() + self.assertEqual(mock_generate.call_count, 1) + self.assertEqual( + "dåtå".encode("utf8"), + (Path(settings.DERIVED_DIR) / "idnits2-rfcs-obsoleted").read_bytes(), + ) + diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index e35938283..429cb4e14 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -181,6 +181,26 @@ class Command(BaseCommand): ) ) + PeriodicTask.objects.get_or_create( + name="Generate idnits2 rfcs-obsoleted blob", + task="ietf.doc.tasks.generate_idnits2_rfcs_obsoleted_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["hourly"], + description="Generate the rfcs-obsoleted file used by idnits", + ), + ) + + PeriodicTask.objects.get_or_create( + name="Generate idnits2 rfc-status blob", + task="ietf.doc.tasks.generate_idnits2_rfc_status_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["hourly"], + description="Generate the rfc_status blob used by idnits", + ), + ) + def show_tasks(self): for label, crontab in self.crontabs.items(): tasks = PeriodicTask.objects.filter(crontab=crontab).order_by( From 48e0aa23f5e9faa45d3ffb4ec0bfb2c8e1924846 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 14 May 2024 20:47:40 -0300 Subject: [PATCH 2/6] refactor: clean up logging (#7419) * fix: log to stdout/stderr in json format * chore: remove UTILS_LOGGER_LEVELS This is not used (there _is_ a setting for the django.security logger in settings_local.py on production, but it is redundant with the settings.LOGGING configuration and is not doing anything). * chore: revert to debug_console django logging * chore: log.log to syslog via datatracker logger * chore: remove unused imports --------- Co-authored-by: Robert Sparks --- ietf/settings.py | 44 ++++++++--------- ietf/utils/log.py | 49 ++----------------- ietf/utils/management/commands/showloggers.py | 13 +---- requirements.txt | 1 + 4 files changed, 28 insertions(+), 79 deletions(-) diff --git a/ietf/settings.py b/ietf/settings.py index cd8c0700a..534c23d0d 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -236,7 +236,7 @@ LOGGING = { # 'loggers': { 'django': { - 'handlers': ['debug_console', 'mail_admins'], + 'handlers': ['debug_console', 'mail_admins',], 'level': 'INFO', }, 'django.request': { @@ -248,13 +248,17 @@ LOGGING = { 'level': 'INFO', }, 'django.security': { - 'handlers': ['debug_console', ], + 'handlers': ['debug_console', ], + 'level': 'INFO', + }, + 'oidc_provider': { + 'handlers': ['debug_console', ], + 'level': 'DEBUG', + }, + 'datatracker': { + 'handlers': ['syslog'], 'level': 'INFO', }, - 'oidc_provider': { - 'handlers': ['debug_console', ], - 'level': 'DEBUG', - }, }, # # No logger filters @@ -263,14 +267,7 @@ LOGGING = { 'console': { 'level': 'DEBUG', 'class': 'logging.StreamHandler', - 'formatter': 'plain', - }, - 'syslog': { - 'level': 'DEBUG', - 'class': 'logging.handlers.SysLogHandler', - 'facility': 'user', - 'formatter': 'plain', - 'address': '/dev/log', + 'formatter': 'json', }, 'debug_console': { # Active only when DEBUG=True @@ -284,6 +281,13 @@ LOGGING = { 'class': 'logging.StreamHandler', 'formatter': 'django.server', }, + 'syslog': { + 'level': 'DEBUG', + 'class': 'logging.handlers.SysLogHandler', + 'facility': 'user', + 'formatter': 'plain', + 'address': '/dev/log', + }, 'mail_admins': { 'level': 'ERROR', 'filters': [ @@ -325,18 +329,12 @@ LOGGING = { 'style': '{', 'format': '{levelname}: {name}:{lineno}: {message}', }, + 'json' : { + '()': 'pythonjsonlogger.jsonlogger.JsonFormatter' + } }, } -# This should be overridden by settings_local for any logger where debug (or -# other) custom log settings are wanted. Use "ietf/manage.py showloggers -l" -# to show registered loggers. The content here should match the levels above -# and is shown as an example: -UTILS_LOGGER_LEVELS: Dict[str, str] = { -# 'django': 'INFO', -# 'django.server': 'INFO', -} - # End logging # ------------------------------------------------------------------------ diff --git a/ietf/utils/log.py b/ietf/utils/log.py index d5a54e551..2a068ade9 100644 --- a/ietf/utils/log.py +++ b/ietf/utils/log.py @@ -9,37 +9,10 @@ import inspect import os.path import traceback -from typing import Callable # pyflakes:ignore - -try: - import syslog - logfunc = syslog.syslog # type: Callable -except ImportError: # import syslog will fail on Windows boxes - logging.basicConfig(filename='tracker.log',level=logging.INFO) - logfunc = logging.info - pass - from django.conf import settings import debug # pyflakes:ignore -formatter = logging.Formatter('{levelname}: {name}:{lineno}: {message}', style='{') -for name, level in settings.UTILS_LOGGER_LEVELS.items(): - logger = logging.getLogger(name) - if not logger.hasHandlers(): - debug.say(' Adding handlers to logger %s' % logger.name) - - handlers = [ - logging.StreamHandler(), - logging.handlers.SysLogHandler(address='/dev/log', - facility=logging.handlers.SysLogHandler.LOG_USER), - ] - for h in handlers: - h.setFormatter(formatter) - h.setLevel(level) - logger.addHandler(h) - debug.say(" Setting %s logging level to %s" % (logger.name, level)) - logger.setLevel(level) def getclass(frame): cls = None @@ -56,20 +29,9 @@ def getcaller(): return (pmodule, pclass, pfunction, pfile, pline) def log(msg, e=None): - "Uses syslog by preference. Logs the given calling point and message." - global logfunc - def _flushfunc(): - pass - _logfunc = logfunc - if settings.SERVER_MODE == 'test': - if getattr(settings, 'show_logging', False) is True: - _logfunc = debug.say - _flushfunc = sys.stdout.flush # pyflakes:ignore (intentional redefinition) - else: + "Logs the given calling point and message to the logging framework's datatracker handler at severity INFO" + if settings.SERVER_MODE == 'test' and not getattr(settings, 'show_logging',False): return - elif settings.DEBUG == True: - _logfunc = debug.say - _flushfunc = sys.stdout.flush # pyflakes:ignore (intentional redefinition) if not isinstance(msg, str): msg = msg.encode('unicode_escape') try: @@ -82,11 +44,8 @@ def log(msg, e=None): where = " in " + func + "()" except IndexError: file, line, where = "/", 0, "" - _flushfunc() - _logfunc("ietf%s(%d)%s: %s" % (file, line, where, msg)) - -logger = logging.getLogger('django') + logging.getLogger("datatracker").info(msg=msg, extra = {"file":file, "line":line, "where":where}) def exc_parts(): @@ -124,6 +83,7 @@ def assertion(statement, state=True, note=None): This acts like an assertion. It uses the django logger in order to send the failed assertion and a backtrace as for an internal server error. """ + logger = logging.getLogger("django") # Note this is a change - before this would have gone to "django" frame = inspect.currentframe().f_back value = eval(statement, frame.f_globals, frame.f_locals) if bool(value) != bool(state): @@ -148,6 +108,7 @@ def assertion(statement, state=True, note=None): def unreachable(date="(unknown)"): "Raises an assertion or sends traceback to admins if executed." + logger = logging.getLogger("django") frame = inspect.currentframe().f_back if settings.DEBUG is True or settings.SERVER_MODE == 'test': raise AssertionError("Arrived at code in %s() which was marked unreachable on %s." % (frame.f_code.co_name, date)) diff --git a/ietf/utils/management/commands/showloggers.py b/ietf/utils/management/commands/showloggers.py index 3de9db0c0..b79da9ce2 100644 --- a/ietf/utils/management/commands/showloggers.py +++ b/ietf/utils/management/commands/showloggers.py @@ -11,18 +11,7 @@ from django.core.management.base import BaseCommand import debug # pyflakes:ignore class Command(BaseCommand): - """ - Display a list or tree representation of python loggers. - - Add a UTILS_LOGGER_LEVELS setting in settings_local.py to configure - non-default logging levels for any registered logger, for instance: - - UTILS_LOGGER_LEVELS = { - 'oicd_provider': 'DEBUG', - 'urllib3.connection': 'DEBUG', - } - - """ + """Display a list or tree representation of python loggers""" help = dedent(__doc__).strip() diff --git a/requirements.txt b/requirements.txt index 94afc9c85..8187c1ceb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -53,6 +53,7 @@ pyopenssl>=22.0.0 # Used by urllib3.contrib, which is used by PyQuery but not pyquery>=1.4.3 python-dateutil>=2.8.2 types-python-dateutil>=2.8.2 +python-json-logger>=2.0.7 python-magic==0.4.18 # Versions beyond the yanked .19 and .20 introduce form failures pymemcache>=4.0.0 # for django.core.cache.backends.memcached.PyMemcacheCache python-mimeparse>=1.6 # from TastyPie From a4e0354090945e9f38eda5a64a96810163d667d7 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 14 May 2024 20:53:31 -0300 Subject: [PATCH 3/6] feat: get tool versions without VersionInfo model (#7418) * feat: get tool versions without VersionInfo model * chore: remove update_external_command_info call * feat: get tool version without VersionInfo * chore: Remove VersionInfo model * chore: Migration to remove VersionInfo * fix: handle errors better; ignore stderr * fix: type annotation --- bin/daily | 3 -- bin/dump-to-names-json | 2 +- ietf/name/fixtures/names.json | 44 ------------------- .../commands/generate_name_fixture.py | 3 +- ietf/submit/checkers.py | 24 ++++++---- ietf/submit/tests.py | 4 +- ietf/utils/__init__.py | 30 ++++++++++++- ietf/utils/admin.py | 7 --- .../commands/update_external_command_info.py | 41 ----------------- .../migrations/0002_delete_versioninfo.py | 16 +++++++ ietf/utils/models.py | 9 ---- ietf/utils/resources.py | 20 +-------- 12 files changed, 66 insertions(+), 137 deletions(-) delete mode 100644 ietf/utils/management/commands/update_external_command_info.py create mode 100644 ietf/utils/migrations/0002_delete_versioninfo.py diff --git a/bin/daily b/bin/daily index 8211e1e23..6557a8922 100755 --- a/bin/daily +++ b/bin/daily @@ -24,9 +24,6 @@ $DTDIR/bin/hourly source $DTDIR/env/bin/activate -# Update our information about the current version of some commands we use -$DTDIR/ietf/manage.py update_external_command_info - # Get IANA-registered yang models #YANG_IANA_DIR=$(python -c 'import ietf.settings; print ietf.settings.SUBMIT_YANG_IANA_MODEL_DIR') # Hardcode the rsync target to avoid any unwanted deletes: diff --git a/bin/dump-to-names-json b/bin/dump-to-names-json index 9c7dfac07..20d4e0f95 100644 --- a/bin/dump-to-names-json +++ b/bin/dump-to-names-json @@ -10,7 +10,7 @@ set -x ietf/manage.py dumpdata --indent 1 doc.State doc.BallotType doc.StateType \ - mailtrigger.MailTrigger mailtrigger.Recipient name utils.VersionInfo \ + mailtrigger.MailTrigger mailtrigger.Recipient name \ group.GroupFeatures stats.CountryAlias dbtemplate.DBTemplate \ | jq --sort-keys "sort_by(.model, .pk)" \ | jq '[.[] | select(.model!="dbtemplate.dbtemplate" or .pk==354)]' > ietf/name/fixtures/names.json diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 35679dcaa..913c6c987 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -16789,49 +16789,5 @@ }, "model": "stats.countryalias", "pk": 303 - }, - { - "fields": { - "command": "xym", - "switch": "--version", - "time": "2024-03-21T07:06:23.405Z", - "used": true, - "version": "xym 0.7.0" - }, - "model": "utils.versioninfo", - "pk": 1 - }, - { - "fields": { - "command": "pyang", - "switch": "--version", - "time": "2024-03-21T07:06:23.755Z", - "used": true, - "version": "pyang 2.6.0" - }, - "model": "utils.versioninfo", - "pk": 2 - }, - { - "fields": { - "command": "yanglint", - "switch": "--version", - "time": "2024-03-21T07:06:23.773Z", - "used": true, - "version": "yanglint SO 1.9.2" - }, - "model": "utils.versioninfo", - "pk": 3 - }, - { - "fields": { - "command": "xml2rfc", - "switch": "--version", - "time": "2024-03-21T07:06:24.609Z", - "used": true, - "version": "xml2rfc 3.20.1" - }, - "model": "utils.versioninfo", - "pk": 4 } ] diff --git a/ietf/name/management/commands/generate_name_fixture.py b/ietf/name/management/commands/generate_name_fixture.py index bbf33e600..ef30e54c7 100644 --- a/ietf/name/management/commands/generate_name_fixture.py +++ b/ietf/name/management/commands/generate_name_fixture.py @@ -77,7 +77,6 @@ class Command(BaseCommand): from ietf.mailtrigger.models import MailTrigger, Recipient from ietf.meeting.models import BusinessConstraint from ietf.stats.models import CountryAlias - from ietf.utils.models import VersionInfo # Grab all ietf.name.models for n in dir(ietf.name.models): @@ -87,7 +86,7 @@ class Command(BaseCommand): model_objects[model_name(item)] = list(item.objects.all().order_by('pk')) for m in ( BallotType, State, StateType, GroupFeatures, MailTrigger, Recipient, - CountryAlias, VersionInfo, BusinessConstraint ): + CountryAlias, BusinessConstraint ): model_objects[model_name(m)] = list(m.objects.all().order_by('pk')) for m in ( DBTemplate, ): diff --git a/ietf/submit/checkers.py b/ietf/submit/checkers.py index 5822f155f..d29e2a235 100644 --- a/ietf/submit/checkers.py +++ b/ietf/submit/checkers.py @@ -14,8 +14,8 @@ from django.conf import settings import debug # pyflakes:ignore +from ietf.utils import tool_version from ietf.utils.log import log, assertion -from ietf.utils.models import VersionInfo from ietf.utils.pipe import pipe from ietf.utils.test_runner import set_coverage_checking @@ -177,8 +177,10 @@ class DraftYangChecker(object): model_list = list(set(model_list)) command = "xym" - cmd_version = VersionInfo.objects.get(command=command).version - message = "%s:\n%s\n\n" % (cmd_version, out.replace('\n\n','\n').strip() if code == 0 else err) + message = "{version}:\n{output}\n\n".format( + version=tool_version[command], + output=out.replace('\n\n', '\n').strip() if code == 0 else err, + ) results.append({ "name": name, @@ -209,7 +211,6 @@ class DraftYangChecker(object): # pyang cmd_template = settings.SUBMIT_PYANG_COMMAND command = [ w for w in cmd_template.split() if not '=' in w ][0] - cmd_version = VersionInfo.objects.get(command=command).version cmd = cmd_template.format(libs=modpath, model=path) venv_path = os.environ.get('VIRTUAL_ENV') or os.path.join(os.getcwd(), 'env') venv_bin = os.path.join(venv_path, 'bin') @@ -238,14 +239,17 @@ class DraftYangChecker(object): except ValueError: pass #passed = passed and code == 0 # For the submission tool. Yang checks always pass - message += "%s: %s:\n%s\n" % (cmd_version, cmd_template, out+"No validation errors\n" if (code == 0 and len(err) == 0) else out+err) + message += "{version}: {template}:\n{output}\n".format( + version=tool_version[command], + template=cmd_template, + output=out + "No validation errors\n" if (code == 0 and len(err) == 0) else out + err, + ) # yanglint set_coverage_checking(False) # we can't count the following as it may or may not be run, depending on setup if settings.SUBMIT_YANGLINT_COMMAND and os.path.exists(settings.YANGLINT_BINARY): cmd_template = settings.SUBMIT_YANGLINT_COMMAND command = [ w for w in cmd_template.split() if not '=' in w ][0] - cmd_version = VersionInfo.objects.get(command=command).version cmd = cmd_template.format(model=path, rfclib=settings.SUBMIT_YANG_RFC_MODEL_DIR, tmplib=workdir, draftlib=settings.SUBMIT_YANG_DRAFT_MODEL_DIR, ianalib=settings.SUBMIT_YANG_IANA_MODEL_DIR, cataloglib=settings.SUBMIT_YANG_CATALOG_MODEL_DIR, ) @@ -264,7 +268,11 @@ class DraftYangChecker(object): except ValueError: pass #passed = passed and code == 0 # For the submission tool. Yang checks always pass - message += "%s: %s:\n%s\n" % (cmd_version, cmd_template, out+"No validation errors\n" if (code == 0 and len(err) == 0) else out+err) + message += "{version}: {template}:\n{output}\n".format( + version=tool_version[command], + template=cmd_template, + output=out + "No validation errors\n" if (code == 0 and len(err) == 0) else out + err, + ) set_coverage_checking(True) else: errors += 1 @@ -293,4 +301,4 @@ class DraftYangChecker(object): items = [ e for res in results for e in res["items"] ] info['items'] = items info['code']['yang'] = model_list - return passed, message, errors, warnings, info \ No newline at end of file + return passed, message, errors, warnings, info diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 08b898c13..58a47aef8 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -49,9 +49,9 @@ from ietf.submit.factories import SubmissionFactory, SubmissionExtResourceFactor from ietf.submit.forms import SubmissionBaseUploadForm, SubmissionAutoUploadForm from ietf.submit.models import Submission, Preapproval, SubmissionExtResource from ietf.submit.tasks import cancel_stale_submissions, process_and_accept_uploaded_submission_task +from ietf.utils import tool_version from ietf.utils.accesstoken import generate_access_token from ietf.utils.mail import outbox, get_payload_text -from ietf.utils.models import VersionInfo from ietf.utils.test_utils import login_testing_unauthorized, TestCase from ietf.utils.timezone import date_today from ietf.utils.draft import PlaintextDraft @@ -1854,7 +1854,7 @@ class SubmitTests(BaseSubmitTestCase): # m = q('#yang-validation-message').text() for command in ['xym', 'pyang', 'yanglint']: - version = VersionInfo.objects.get(command=command).version + version = tool_version[command] if command != 'yanglint' or (settings.SUBMIT_YANGLINT_COMMAND and os.path.exists(settings.YANGLINT_BINARY)): self.assertIn(version, m) self.assertIn("draft-yang-testing-invalid-00.txt", m) diff --git a/ietf/utils/__init__.py b/ietf/utils/__init__.py index 7f1df9760..fbe55eb04 100644 --- a/ietf/utils/__init__.py +++ b/ietf/utils/__init__.py @@ -1 +1,29 @@ -# Copyright The IETF Trust 2007, All Rights Reserved +# Copyright The IETF Trust 2007-2024, All Rights Reserved +import subprocess + + +class _ToolVersionManager: + _known = [ + "pyang", + "xml2rfc", + "xym", + "yanglint", + ] + _versions: dict[str, str] = dict() + + def __getitem__(self, item): + if item not in self._known: + return "Unknown" + elif item not in self._versions: + try: + self._versions[item] = subprocess.run( + [item, "--version"], + capture_output=True, + check=True, + ).stdout.decode().strip() + except subprocess.CalledProcessError: + return "Unknown" + return self._versions[item] + + +tool_version = _ToolVersionManager() diff --git a/ietf/utils/admin.py b/ietf/utils/admin.py index fa1ebb708..6c1c8726e 100644 --- a/ietf/utils/admin.py +++ b/ietf/utils/admin.py @@ -5,8 +5,6 @@ from django.contrib import admin from django.utils.encoding import force_str -from ietf.utils.models import VersionInfo - def name(obj): if hasattr(obj, 'abbrev'): return obj.abbrev() @@ -58,8 +56,3 @@ class DumpInfoAdmin(admin.ModelAdmin): list_display = ['date', 'host', 'tz'] list_filter = ['date'] admin.site.register(DumpInfo, DumpInfoAdmin) - -class VersionInfoAdmin(admin.ModelAdmin): - list_display = ['command', 'switch', 'version', 'time', ] -admin.site.register(VersionInfo, VersionInfoAdmin) - diff --git a/ietf/utils/management/commands/update_external_command_info.py b/ietf/utils/management/commands/update_external_command_info.py deleted file mode 100644 index e9e24f000..000000000 --- a/ietf/utils/management/commands/update_external_command_info.py +++ /dev/null @@ -1,41 +0,0 @@ -# Copyright The IETF Trust 2017-2020, All Rights Reserved -# -*- coding: utf-8 -*- - - -import sys - -from textwrap import dedent - -from django.core.management.base import BaseCommand - -import debug # pyflakes:ignore - -from ietf.utils.models import VersionInfo -from ietf.utils.pipe import pipe - -class Command(BaseCommand): - """ - Update the version information for external commands used by the datatracker. - - Iterates through the entries in the VersionInfo table, runs the relevant - command, and updates the version string with the result. - - """ - - help = dedent(__doc__).strip() - - def handle(self, *filenames, **options): - for c in VersionInfo.objects.filter(used=True): - cmd = "%s %s" % (c.command, c.switch) - code, out, err = pipe(cmd) - out = out.decode('utf-8') - err = err.decode('utf-8') - if code != 0: - sys.stderr.write("Command '%s' returned %s: \n%s\n%s\n" % (cmd, code, out, err)) - else: - c.version = (out.strip()+'\n'+err.strip()).strip() - if options.get('verbosity', 1) > 1: - sys.stdout.write( - "Command: %s\n" - " Version: %s\n" % (cmd, c.version)) - c.save() diff --git a/ietf/utils/migrations/0002_delete_versioninfo.py b/ietf/utils/migrations/0002_delete_versioninfo.py new file mode 100644 index 000000000..2835bb017 --- /dev/null +++ b/ietf/utils/migrations/0002_delete_versioninfo.py @@ -0,0 +1,16 @@ +# Generated by Django 4.2.11 on 2024-05-03 21:03 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("utils", "0001_initial"), + ] + + operations = [ + migrations.DeleteModel( + name="VersionInfo", + ), + ] diff --git a/ietf/utils/models.py b/ietf/utils/models.py index 0915537fd..21af5766e 100644 --- a/ietf/utils/models.py +++ b/ietf/utils/models.py @@ -9,15 +9,6 @@ class DumpInfo(models.Model): host = models.CharField(max_length=128) tz = models.CharField(max_length=32, default='UTC') -class VersionInfo(models.Model): - time = models.DateTimeField(auto_now=True) - command = models.CharField(max_length=32) - switch = models.CharField(max_length=16) - version = models.CharField(max_length=64) - used = models.BooleanField(default=True) - class Meta: - verbose_name_plural = 'VersionInfo' - class ForeignKey(models.ForeignKey): "A local ForeignKey proxy which provides the on_delete value required under Django 2.0." def __init__(self, to, on_delete=models.CASCADE, **kwargs): diff --git a/ietf/utils/resources.py b/ietf/utils/resources.py index 6d61c5e2e..1252cfef1 100644 --- a/ietf/utils/resources.py +++ b/ietf/utils/resources.py @@ -12,7 +12,7 @@ from django.contrib.auth.models import User from django.contrib.contenttypes.models import ContentType from ietf import api -from ietf.utils.models import DumpInfo, VersionInfo +from ietf.utils.models import DumpInfo class UserResource(ModelResource): @@ -43,21 +43,3 @@ class DumpInfoResource(ModelResource): "host": ALL, } api.utils.register(DumpInfoResource()) - - -class VersionInfoResource(ModelResource): - class Meta: - queryset = VersionInfo.objects.all() - serializer = api.Serializer() - cache = SimpleCache() - #resource_name = 'versioninfo' - ordering = ['id', ] - filtering = { - "id": ALL, - "time": ALL, - "command": ALL, - "switch": ALL, - "version": ALL, - "used": ALL, - } -api.utils.register(VersionInfoResource()) From 46a00acefc249ac879a4c1cb876c9a1b5fd2ed98 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 14 May 2024 20:56:14 -0300 Subject: [PATCH 4/6] refactor: sync to RFC Editor queue via celery (#7415) * feat: rfc_editor_queue_updates_task * refactor: use rfc_editor_queue_updates_task() * chore: remove now-unused scripts * test: test new task * chore: de-lint --- ietf/bin/rfc-editor-index-updates | 110 ------------------------------ ietf/bin/rfc-editor-queue-updates | 44 ------------ ietf/sync/tasks.py | 28 ++++++++ ietf/sync/tests.py | 30 ++++++++ ietf/sync/views.py | 19 +----- 5 files changed, 61 insertions(+), 170 deletions(-) delete mode 100755 ietf/bin/rfc-editor-index-updates delete mode 100755 ietf/bin/rfc-editor-queue-updates diff --git a/ietf/bin/rfc-editor-index-updates b/ietf/bin/rfc-editor-index-updates deleted file mode 100755 index c3e8f1f46..000000000 --- a/ietf/bin/rfc-editor-index-updates +++ /dev/null @@ -1,110 +0,0 @@ -#!/usr/bin/env python - -# This script requires that the proper virtual python environment has been -# invoked before start - -import datetime -import io -import os -import requests -import sys -import syslog -import traceback - -# boilerplate -basedir = os.path.abspath(os.path.join(os.path.dirname(__file__), "../..")) -sys.path = [ basedir ] + sys.path -os.environ["DJANGO_SETTINGS_MODULE"] = "ietf.settings" - -# Before invoking django -syslog.openlog(os.path.basename(__file__), syslog.LOG_PID, syslog.LOG_USER) - -import django -django.setup() - -from django.conf import settings -from optparse import OptionParser -from django.core.mail import mail_admins - -from ietf.doc.utils import rebuild_reference_relations -from ietf.utils.log import log -from ietf.utils.pipe import pipe -from ietf.utils.timezone import date_today - -import ietf.sync.rfceditor - - -parser = OptionParser() -parser.add_option("-d", dest="skip_date", - help="To speed up processing skip RFCs published before this date (default is one year ago)", metavar="YYYY-MM-DD") - -options, args = parser.parse_args() - -skip_date = date_today() - datetime.timedelta(days=365) -if options.skip_date: - skip_date = datetime.datetime.strptime(options.skip_date, "%Y-%m-%d").date() - -log("Updating document metadata from RFC index going back to %s, from %s" % (skip_date, settings.RFC_EDITOR_INDEX_URL)) - - -try: - response = requests.get( - settings.RFC_EDITOR_INDEX_URL, - timeout=30, # seconds - ) -except requests.Timeout as exc: - log(f'GET request timed out retrieving RFC editor index: {exc}') - sys.exit(1) - - -rfc_index_xml = response.text -index_data = ietf.sync.rfceditor.parse_index(io.StringIO(rfc_index_xml)) - -try: - response = requests.get( - settings.RFC_EDITOR_ERRATA_JSON_URL, - timeout=30, # seconds - ) -except requests.Timeout as exc: - log(f'GET request timed out retrieving RFC editor errata: {exc}') - sys.exit(1) -errata_data = response.json() - -if len(index_data) < ietf.sync.rfceditor.MIN_INDEX_RESULTS: - log("Not enough index entries, only %s" % len(index_data)) - sys.exit(1) - -if len(errata_data) < ietf.sync.rfceditor.MIN_ERRATA_RESULTS: - log("Not enough errata entries, only %s" % len(errata_data)) - sys.exit(1) - -new_rfcs = [] -for rfc_number, changes, doc, rfc_published in ietf.sync.rfceditor.update_docs_from_rfc_index(index_data, errata_data, skip_older_than_date=skip_date): - if rfc_published: - new_rfcs.append(doc) - - for c in changes: - log("RFC%s, %s: %s" % (rfc_number, doc.name, c)) - -sys.exit(0) - -# This can be called while processing a notifying POST from the RFC Editor -# Spawn a child to sync the rfcs and calculate new reference relationships -# so that the POST - -newpid = os.fork() - -if newpid == 0: - try: - pipe("%s -a %s %s" % (settings.RSYNC_BINARY,settings.RFC_TEXT_RSYNC_SOURCE,settings.RFC_PATH)) - for rfc in new_rfcs: - rebuild_reference_relations(rfc) - log("Updated references for %s"%rfc.name) - except: - subject = "Exception in updating references for new rfcs: %s : %s" % (sys.exc_info()[0],sys.exc_info()[1]) - msg = "%s\n%s\n----\n%s"%(sys.exc_info()[0],sys.exc_info()[1],traceback.format_tb(sys.exc_info()[2])) - mail_admins(subject,msg,fail_silently=True) - log(subject) - os._exit(0) -else: - sys.exit(0) diff --git a/ietf/bin/rfc-editor-queue-updates b/ietf/bin/rfc-editor-queue-updates deleted file mode 100755 index b441e50eb..000000000 --- a/ietf/bin/rfc-editor-queue-updates +++ /dev/null @@ -1,44 +0,0 @@ -#!/usr/bin/env python - -import io -import os -import requests -import sys - -# boilerplate -basedir = os.path.abspath(os.path.join(os.path.dirname(__file__), "../..")) -sys.path = [ basedir ] + sys.path -os.environ["DJANGO_SETTINGS_MODULE"] = "ietf.settings" - -import django -django.setup() - -from django.conf import settings - -from ietf.sync.rfceditor import parse_queue, MIN_QUEUE_RESULTS, update_drafts_from_queue -from ietf.utils.log import log - -log("Updating RFC Editor queue states from %s" % settings.RFC_EDITOR_QUEUE_URL) - -try: - response = requests.get( - settings.RFC_EDITOR_QUEUE_URL, - timeout=30, # seconds - ) -except requests.Timeout as exc: - log(f'GET request timed out retrieving RFC editor queue: {exc}') - sys.exit(1) -drafts, warnings = parse_queue(io.StringIO(response.text)) -for w in warnings: - log(u"Warning: %s" % w) - -if len(drafts) < MIN_QUEUE_RESULTS: - log("Not enough results, only %s" % len(drafts)) - sys.exit(1) - -changed, warnings = update_drafts_from_queue(drafts) -for w in warnings: - log(u"Warning: %s" % w) - -for c in changed: - log(u"Updated %s" % c) diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py index bc1218601..53e23d791 100644 --- a/ietf/sync/tasks.py +++ b/ietf/sync/tasks.py @@ -13,6 +13,7 @@ from django.utils import timezone from ietf.sync import iana from ietf.sync import rfceditor +from ietf.sync.rfceditor import MIN_QUEUE_RESULTS, parse_queue, update_drafts_from_queue from ietf.utils import log from ietf.utils.timezone import date_today @@ -70,6 +71,33 @@ def rfc_editor_index_update_task(full_index=False): log.log("RFC%s, %s: %s" % (rfc_number, doc.name, c)) +@shared_task +def rfc_editor_queue_updates_task(): + log.log(f"Updating RFC Editor queue states from {settings.RFC_EDITOR_QUEUE_URL}") + try: + response = requests.get( + settings.RFC_EDITOR_QUEUE_URL, + timeout=30, # seconds + ) + except requests.Timeout as exc: + log.log(f"GET request timed out retrieving RFC editor queue: {exc}") + return # failed + drafts, warnings = parse_queue(io.StringIO(response.text)) + for w in warnings: + log.log(f"Warning: {w}") + + if len(drafts) < MIN_QUEUE_RESULTS: + log.log("Not enough results, only %s" % len(drafts)) + return # failed + + changed, warnings = update_drafts_from_queue(drafts) + for w in warnings: + log.log(f"Warning: {w}") + + for c in changed: + log.log(f"Updated {c}") + + @shared_task def iana_changes_update_task(): # compensate to avoid we ask for something that happened now and then diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py index db5619095..b0cdf863f 100644 --- a/ietf/sync/tests.py +++ b/ietf/sync/tests.py @@ -886,6 +886,36 @@ class TaskTests(TestCase): tasks.rfc_editor_index_update_task(full_index=False) self.assertFalse(update_docs_mock.called) + @override_settings(RFC_EDITOR_QUEUE_URL="https://rfc-editor.example.com/queue/") + @mock.patch("ietf.sync.tasks.update_drafts_from_queue") + @mock.patch("ietf.sync.tasks.parse_queue") + def test_rfc_editor_queue_updates_task(self, mock_parse, mock_update): + # test a request timeout + self.requests_mock.get("https://rfc-editor.example.com/queue/", exc=requests.exceptions.Timeout) + tasks.rfc_editor_queue_updates_task() + self.assertFalse(mock_parse.called) + self.assertFalse(mock_update.called) + + # now return a value rather than an exception + self.requests_mock.get("https://rfc-editor.example.com/queue/", text="the response") + + # mock returning < MIN_QUEUE_RESULTS values - treated as an error, so no update takes place + mock_parse.return_value = ([n for n in range(rfceditor.MIN_QUEUE_RESULTS - 1)], ["a warning"]) + tasks.rfc_editor_queue_updates_task() + self.assertEqual(mock_parse.call_count, 1) + self.assertEqual(mock_parse.call_args[0][0].read(), "the response") + self.assertFalse(mock_update.called) + mock_parse.reset_mock() + + # mock returning +. MIN_QUEUE_RESULTS - should succeed + mock_parse.return_value = ([n for n in range(rfceditor.MIN_QUEUE_RESULTS)], ["a warning"]) + mock_update.return_value = ([1,2,3], ["another warning"]) + tasks.rfc_editor_queue_updates_task() + self.assertEqual(mock_parse.call_count, 1) + self.assertEqual(mock_parse.call_args[0][0].read(), "the response") + self.assertEqual(mock_update.call_count, 1) + self.assertEqual(mock_update.call_args, mock.call([n for n in range(rfceditor.MIN_QUEUE_RESULTS)])) + @override_settings(IANA_SYNC_CHANGES_URL="https://iana.example.com/sync/") @mock.patch("ietf.sync.tasks.iana.update_history_with_changes") @mock.patch("ietf.sync.tasks.iana.parse_changes_json") diff --git a/ietf/sync/views.py b/ietf/sync/views.py index 788e982f7..da407e1ef 100644 --- a/ietf/sync/views.py +++ b/ietf/sync/views.py @@ -2,7 +2,6 @@ # -*- coding: utf-8 -*- import datetime -import subprocess import os import json @@ -79,30 +78,18 @@ def notify(request, org, notification): raise Http404 if request.method == "POST": - def runscript(name): - python = os.path.join(os.path.dirname(settings.BASE_DIR), "env", "bin", "python") - cmd = [python, os.path.join(SYNC_BIN_PATH, name)] - cmdstring = " ".join(cmd) - p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - out, err = p.communicate() - out = out.decode('utf-8') - err = err.decode('utf-8') - if p.returncode: - log("Subprocess error %s when running '%s': %s %s" % (p.returncode, cmd, err, out)) - raise subprocess.CalledProcessError(p.returncode, cmdstring, "\n".join([err, out])) - if notification == "index": log("Queuing RFC Editor index sync from notify view POST") tasks.rfc_editor_index_update_task.delay() + elif notification == "queue": + log("Queuing RFC Editor queue sync from notify view POST") + tasks.rfc_editor_queue_updates_task.delay() elif notification == "changes": log("Queuing IANA changes sync from notify view POST") tasks.iana_changes_update_task.delay() elif notification == "protocols": log("Queuing IANA protocols sync from notify view POST") tasks.iana_protocols_update_task.delay() - elif notification == "queue": - log("Running sync script from notify view POST") - runscript("rfc-editor-queue-updates") return HttpResponse("OK", content_type="text/plain; charset=%s"%settings.DEFAULT_CHARSET) From c9f35987bce1ee50d8bb88b24b5c7d8bb6b855b5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 15 May 2024 14:04:47 -0300 Subject: [PATCH 5/6] refactor: expire last calls via celery (#7417) * feat: expire_last_calls_task * feat: create periodic task for last calls * test: test new task * chore: remove expire-last-calls script --- bin/daily | 4 --- ietf/bin/expire-last-calls | 34 ------------------- ietf/doc/tasks.py | 12 +++++++ ietf/doc/tests_tasks.py | 25 +++++++++++++- .../management/commands/periodic_tasks.py | 10 ++++++ 5 files changed, 46 insertions(+), 39 deletions(-) delete mode 100755 ietf/bin/expire-last-calls diff --git a/bin/daily b/bin/daily index 6557a8922..997b9c7d7 100755 --- a/bin/daily +++ b/bin/daily @@ -40,9 +40,5 @@ $DTDIR/ietf/manage.py populate_yang_model_dirs -v0 # Re-run yang checks on active documents $DTDIR/ietf/manage.py run_yang_model_checks -v0 -# Expire last calls -# Enable when removed from /a/www/ietf-datatracker/scripts/Cron-runner: -$DTDIR/ietf/bin/expire-last-calls - # Purge older PersonApiKeyEvents $DTDIR/ietf/manage.py purge_old_personal_api_key_events 14 diff --git a/ietf/bin/expire-last-calls b/ietf/bin/expire-last-calls deleted file mode 100755 index 83b565e19..000000000 --- a/ietf/bin/expire-last-calls +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/bin/env python - -# This script requires that the proper virtual python environment has been -# invoked before start - -import os -import sys -import syslog - -# boilerplate -basedir = os.path.abspath(os.path.join(os.path.dirname(__file__), "../..")) -sys.path = [ basedir ] + sys.path -os.environ["DJANGO_SETTINGS_MODULE"] = "ietf.settings" - -virtualenv_activation = os.path.join(basedir, "env", "bin", "activate_this.py") -if os.path.exists(virtualenv_activation): - execfile(virtualenv_activation, dict(__file__=virtualenv_activation)) - -syslog.openlog(os.path.basename(__file__), syslog.LOG_PID, syslog.LOG_USER) - -import django -django.setup() - -# ---------------------------------------------------------------------- - -from ietf.doc.lastcall import get_expired_last_calls, expire_last_call - -drafts = get_expired_last_calls() -for doc in drafts: - try: - expire_last_call(doc) - syslog.syslog("Expired last call for %s (id=%s)" % (doc.file_tag(), doc.pk)) - except Exception as e: - syslog.syslog(syslog.LOG_ERR, "ERROR: Failed to expire last call for %s (id=%s)" % (doc.file_tag(), doc.pk)) diff --git a/ietf/doc/tasks.py b/ietf/doc/tasks.py index b189a6827..d1cf6656a 100644 --- a/ietf/doc/tasks.py +++ b/ietf/doc/tasks.py @@ -23,6 +23,7 @@ from .expire import ( get_soon_to_expire_drafts, send_expire_warning_for_draft, ) +from .lastcall import get_expired_last_calls, expire_last_call from .models import Document from .utils import generate_idnits2_rfc_status, generate_idnits2_rfcs_obsoleted @@ -61,6 +62,17 @@ def notify_expirations_task(notify_days=14): @shared_task +def expire_last_calls_task(): + for doc in get_expired_last_calls(): + try: + expire_last_call(doc) + except Exception: + log.log(f"ERROR: Failed to expire last call for {doc.file_tag()} (id={doc.pk})") + else: + log.log(f"Expired last call for {doc.file_tag()} (id={doc.pk})") + + +@shared_task def generate_idnits2_rfc_status_task(): outpath = Path(settings.DERIVED_DIR) / "idnits2-rfc-status" blob = generate_idnits2_rfc_status() diff --git a/ietf/doc/tests_tasks.py b/ietf/doc/tests_tasks.py index 8915a6c5a..3eeae2b34 100644 --- a/ietf/doc/tests_tasks.py +++ b/ietf/doc/tests_tasks.py @@ -12,6 +12,7 @@ from .factories import DocumentFactory from .models import Document from .tasks import ( expire_ids_task, + expire_last_calls_task, generate_idnits2_rfcs_obsoleted_task, generate_idnits2_rfc_status_task, notify_expirations_task, @@ -71,6 +72,29 @@ class TaskTests(TestCase): self.assertEqual(send_warning_mock.call_count, 1) self.assertEqual(send_warning_mock.call_args[0], ("sentinel",)) + @mock.patch("ietf.doc.tasks.expire_last_call") + @mock.patch("ietf.doc.tasks.get_expired_last_calls") + def test_expire_last_calls_task(self, mock_get_expired, mock_expire): + docs = DocumentFactory.create_batch(3) + mock_get_expired.return_value = docs + expire_last_calls_task() + self.assertTrue(mock_get_expired.called) + self.assertEqual(mock_expire.call_count, 3) + self.assertEqual(mock_expire.call_args_list[0], mock.call(docs[0])) + self.assertEqual(mock_expire.call_args_list[1], mock.call(docs[1])) + self.assertEqual(mock_expire.call_args_list[2], mock.call(docs[2])) + + # Check that it runs even if exceptions occur + mock_get_expired.reset_mock() + mock_expire.reset_mock() + mock_expire.side_effect = ValueError + expire_last_calls_task() + self.assertTrue(mock_get_expired.called) + self.assertEqual(mock_expire.call_count, 3) + self.assertEqual(mock_expire.call_args_list[0], mock.call(docs[0])) + self.assertEqual(mock_expire.call_args_list[1], mock.call(docs[1])) + self.assertEqual(mock_expire.call_args_list[2], mock.call(docs[2])) + @mock.patch("ietf.doc.tasks.generate_idnits2_rfc_status") def test_generate_idnits2_rfc_status_task(self, mock_generate): mock_generate.return_value = "dåtå" @@ -90,4 +114,3 @@ class TaskTests(TestCase): "dåtå".encode("utf8"), (Path(settings.DERIVED_DIR) / "idnits2-rfcs-obsoleted").read_bytes(), ) - diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index 429cb4e14..a4357cab1 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -141,6 +141,16 @@ class Command(BaseCommand): ), ) + PeriodicTask.objects.get_or_create( + name="Expire Last Calls", + task="ietf.doc.tasks.expire_last_calls_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["daily"], + description="Move docs whose last call has expired to their next states", + ), + ) + PeriodicTask.objects.get_or_create( name="Sync with IANA changes", task="ietf.sync.tasks.iana_changes_update_task", From c59d6122d9ae9f9770ac6d6273cc36a238849b85 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 15 May 2024 15:25:15 -0300 Subject: [PATCH 6/6] refactor: send_nomcom_reminders via celery task (#7424) * refactor: send_reminders.py -> celery task * chore: add PeriodicTask * chore: remove management command and tests --- .../management/commands/send_reminders.py | 37 ----------------- ietf/nomcom/tasks.py | 10 +++++ ietf/nomcom/tests.py | 40 ++++++++++++------- ietf/nomcom/utils.py | 24 +++++++++++ .../management/commands/periodic_tasks.py | 10 +++++ 5 files changed, 70 insertions(+), 51 deletions(-) delete mode 100644 ietf/nomcom/management/commands/send_reminders.py create mode 100644 ietf/nomcom/tasks.py diff --git a/ietf/nomcom/management/commands/send_reminders.py b/ietf/nomcom/management/commands/send_reminders.py deleted file mode 100644 index bc1042543..000000000 --- a/ietf/nomcom/management/commands/send_reminders.py +++ /dev/null @@ -1,37 +0,0 @@ -# Copyright The IETF Trust 2013-2020, All Rights Reserved -# -*- coding: utf-8 -*- - - -import syslog - -from django.core.management.base import BaseCommand - -from ietf.nomcom.models import NomCom, NomineePosition -from ietf.nomcom.utils import send_accept_reminder_to_nominee,send_questionnaire_reminder_to_nominee -from ietf.utils.timezone import date_today - - -def log(message): - syslog.syslog(message) - -def is_time_to_send(nomcom,send_date,nomination_date): - if nomcom.reminder_interval: - days_passed = (send_date - nomination_date).days - return days_passed > 0 and days_passed % nomcom.reminder_interval == 0 - else: - return bool(nomcom.reminderdates_set.filter(date=send_date)) - -class Command(BaseCommand): - help = ("Send acceptance and questionnaire reminders to nominees") - - def handle(self, *args, **options): - for nomcom in NomCom.objects.filter(group__state__slug='active'): - nps = NomineePosition.objects.filter(nominee__nomcom=nomcom,nominee__duplicated__isnull=True) - for nominee_position in nps.pending(): - if is_time_to_send(nomcom, date_today(), nominee_position.time.date()): - send_accept_reminder_to_nominee(nominee_position) - log('Sent accept reminder to %s' % nominee_position.nominee.email.address) - for nominee_position in nps.accepted().without_questionnaire_response(): - if is_time_to_send(nomcom, date_today(), nominee_position.time.date()): - send_questionnaire_reminder_to_nominee(nominee_position) - log('Sent questionnaire reminder to %s' % nominee_position.nominee.email.address) diff --git a/ietf/nomcom/tasks.py b/ietf/nomcom/tasks.py new file mode 100644 index 000000000..3d063a6b2 --- /dev/null +++ b/ietf/nomcom/tasks.py @@ -0,0 +1,10 @@ +# Copyright The IETF Trust 2024, All Rights Reserved + +from celery import shared_task + +from .utils import send_reminders + + +@shared_task +def send_nomcom_reminders_task(): + send_reminders() diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index 9a615c91d..8f94cc7fc 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -40,14 +40,14 @@ from ietf.nomcom.models import NomineePosition, Position, Nominee, \ NomineePositionStateName, Feedback, FeedbackTypeName, \ Nomination, FeedbackLastSeen, TopicFeedbackLastSeen, ReminderDates, \ NomCom -from ietf.nomcom.management.commands.send_reminders import Command, is_time_to_send from ietf.nomcom.factories import NomComFactory, FeedbackFactory, TopicFactory, \ nomcom_kwargs_for_year, provide_private_key_to_test_client, \ key +from ietf.nomcom.tasks import send_nomcom_reminders_task from ietf.nomcom.utils import get_nomcom_by_year, make_nomineeposition, \ get_hash_nominee_position, is_eligible, list_eligible, \ get_eligibility_date, suggest_affiliation, ingest_feedback_email, \ - decorate_volunteers_with_qualifications + decorate_volunteers_with_qualifications, send_reminders, _is_time_to_send_reminder from ietf.person.factories import PersonFactory, EmailFactory from ietf.person.models import Email, Person from ietf.stats.models import MeetingRegistration @@ -1207,36 +1207,41 @@ class ReminderTest(TestCase): teardown_test_public_keys_dir(self) super().tearDown() - def test_is_time_to_send(self): + def test_is_time_to_send_reminder(self): self.nomcom.reminder_interval = 4 today = date_today() - self.assertTrue(is_time_to_send(self.nomcom,today+datetime.timedelta(days=4),today)) + self.assertTrue( + _is_time_to_send_reminder(self.nomcom, today + datetime.timedelta(days=4), today) + ) for delta in range(4): - self.assertFalse(is_time_to_send(self.nomcom,today+datetime.timedelta(days=delta),today)) + self.assertFalse( + _is_time_to_send_reminder( + self.nomcom, today + datetime.timedelta(days=delta), today + ) + ) self.nomcom.reminder_interval = None - self.assertFalse(is_time_to_send(self.nomcom,today,today)) + self.assertFalse(_is_time_to_send_reminder(self.nomcom, today, today)) self.nomcom.reminderdates_set.create(date=today) - self.assertTrue(is_time_to_send(self.nomcom,today,today)) + self.assertTrue(_is_time_to_send_reminder(self.nomcom, today, today)) - def test_command(self): - c = Command() - messages_before=len(outbox) + def test_send_reminders(self): + messages_before = len(outbox) self.nomcom.reminder_interval = 3 self.nomcom.save() - c.handle(None,None) + send_reminders() self.assertEqual(len(outbox), messages_before + 2) self.assertIn('nominee1@example.org', outbox[-1]['To']) self.assertIn('please complete', outbox[-1]['Subject']) self.assertIn('nominee1@example.org', outbox[-2]['To']) self.assertIn('please accept', outbox[-2]['Subject']) - messages_before=len(outbox) + messages_before = len(outbox) self.nomcom.reminder_interval = 4 self.nomcom.save() - c.handle(None,None) + send_reminders() self.assertEqual(len(outbox), messages_before + 1) self.assertIn('nominee2@example.org', outbox[-1]['To']) self.assertIn('please accept', outbox[-1]['Subject']) - + def test_remind_accept_view(self): url = reverse('ietf.nomcom.views.send_reminder_mail', kwargs={'year': NOMCOM_YEAR,'type':'accept'}) login_testing_unauthorized(self, CHAIR_USER, url) @@ -3048,3 +3053,10 @@ class ReclassifyFeedbackTests(TestCase): self.assertEqual(fb.type_id, 'junk') self.assertEqual(Feedback.objects.filter(type='read').count(), 0) self.assertEqual(Feedback.objects.filter(type='junk').count(), 1) + + +class TaskTests(TestCase): + @mock.patch("ietf.nomcom.tasks.send_reminders") + def test_send_nomcom_reminders_task(self, mock_send): + send_nomcom_reminders_task() + self.assertEqual(mock_send.call_count, 1) diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 53e775deb..ab155ef1d 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -747,3 +747,27 @@ def ingest_feedback_email(message: bytes, year: int): email_original_message=message, ) from err log("Received nomcom email from %s" % feedback.author) + + +def _is_time_to_send_reminder(nomcom, send_date, nomination_date): + if nomcom.reminder_interval: + days_passed = (send_date - nomination_date).days + return days_passed > 0 and days_passed % nomcom.reminder_interval == 0 + else: + return bool(nomcom.reminderdates_set.filter(date=send_date)) + + +def send_reminders(): + from .models import NomCom, NomineePosition + for nomcom in NomCom.objects.filter(group__state__slug="active"): + nps = NomineePosition.objects.filter( + nominee__nomcom=nomcom, nominee__duplicated__isnull=True + ) + for nominee_position in nps.pending(): + if _is_time_to_send_reminder(nomcom, date_today(), nominee_position.time.date()): + send_accept_reminder_to_nominee(nominee_position) + log(f"Sent accept reminder to {nominee_position.nominee.email.address}") + for nominee_position in nps.accepted().without_questionnaire_response(): + if _is_time_to_send_reminder(nomcom, date_today(), nominee_position.time.date()): + send_questionnaire_reminder_to_nominee(nominee_position) + log(f"Sent questionnaire reminder to {nominee_position.nominee.email.address}") diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index a4357cab1..52f4932ed 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -211,6 +211,16 @@ class Command(BaseCommand): ), ) + PeriodicTask.objects.get_or_create( + name="Send NomCom reminders", + task="ietf.nomcom.tasks.send_nomcom_reminders_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["daily"], + description="Send acceptance and questionnaire reminders to nominees", + ), + ) + def show_tasks(self): for label, crontab in self.crontabs.items(): tasks = PeriodicTask.objects.filter(crontab=crontab).order_by(