Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions bin/daily
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion bin/dump-to-names-json
Original file line number Diff line number Diff line change
Expand Up @@ -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
44 changes: 0 additions & 44 deletions ietf/name/fixtures/names.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]
3 changes: 1 addition & 2 deletions ietf/name/management/commands/generate_name_fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ def output(seq):
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):
Expand All @@ -87,7 +86,7 @@ def output(seq):
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, ):
Expand Down
24 changes: 16 additions & 8 deletions ietf/submit/checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

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

Expand Down Expand Up @@ -177,8 +177,10 @@ def check_file_txt(self, path):
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,
Expand Down Expand Up @@ -209,7 +211,6 @@ def check_file_txt(self, path):
# 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')
Expand Down Expand Up @@ -238,14 +239,17 @@ def check_file_txt(self, path):
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, )
Expand All @@ -264,7 +268,11 @@ def check_file_txt(self, path):
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
Expand Down Expand Up @@ -293,4 +301,4 @@ def check_file_txt(self, path):
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
return passed, message, errors, warnings, info
4 changes: 2 additions & 2 deletions ietf/submit/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@
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
Expand Down Expand Up @@ -1854,7 +1854,7 @@ def test_submit_invalid_yang(self):
#
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)
Expand Down
30 changes: 29 additions & 1 deletion ietf/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -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(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thought that occurs to me is that this puts the time to invoke these 4 things (which should be fast) in the warmup path of every worker thread.

I'm willing to give this a shot, but if we're not already planning to do so in feat/k8s, maybe this should be replaced with something that happens at build time?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, moving to a build time mechanism would be better. I expect that since we generally (or maybe always) actually do work with the tool we ask for the version of, any overhead will be really small by comparison. I guess that's not guaranteed though.

This has the virtue of working both on ietfa and in a container, so it'll bridge the move to the new infrastructure.

[item, "--version"],
capture_output=True,
check=True,
).stdout.decode().strip()
except subprocess.CalledProcessError:
return "Unknown"
return self._versions[item]


tool_version = _ToolVersionManager()
7 changes: 0 additions & 7 deletions ietf/utils/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)

41 changes: 0 additions & 41 deletions ietf/utils/management/commands/update_external_command_info.py

This file was deleted.

16 changes: 16 additions & 0 deletions ietf/utils/migrations/0002_delete_versioninfo.py
Original file line number Diff line number Diff line change
@@ -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",
),
]
9 changes: 0 additions & 9 deletions ietf/utils/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
20 changes: 1 addition & 19 deletions ietf/utils/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
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):
Expand Down Expand Up @@ -43,21 +43,3 @@ class Meta:
"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())