From 47cc6e1367a3b31955671656f4834ecc422315d5 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Fri, 4 Oct 2024 09:58:27 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=8C=20Add=20`needs=5Fuml=5Fprocess=5Fm?= =?UTF-8?q?ax=5Ftime`=20configuration=20(#1314)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To guard against long-running `needuml` / `needarch` processing times. In particular, these can occur if the jinja template contains many calls to the filter function. --- docs/configuration.rst | 9 +++++++++ docs/filter.rst | 1 + sphinx_needs/config.py | 6 +++++- sphinx_needs/data.py | 2 ++ sphinx_needs/directives/needuml.py | 23 +++++++++++++++++++++-- tests/test_needarch.py | 5 +++-- tests/test_needuml.py | 15 +++++++++------ 7 files changed, 50 insertions(+), 11 deletions(-) diff --git a/docs/configuration.rst b/docs/configuration.rst index ec31f60d5..4957108a9 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -542,6 +542,15 @@ needs_filter_max_time If set, warn if any :ref:`filter processing ` call takes longer than the given time in seconds. +.. _needs_uml_process_max_time: + +needs_uml_process_max_time +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: 4.0.0 + +If set, warn if any :ref:`needuml` or :ref:`needarch` jinja content rendering takes longer than the given time in seconds. + .. _needs_flow_engine: needs_flow_engine diff --git a/docs/filter.rst b/docs/filter.rst index 05dcba849..656d1483c 100644 --- a/docs/filter.rst +++ b/docs/filter.rst @@ -213,6 +213,7 @@ Also filters containing ``and`` will be split into multiple filters and evaluate For example, ``type == 'spec' and other == 'value'`` will first be filtered performantly by ``type == 'spec'`` and then the remaining needs will be filtered by ``other == 'value'``. To guard against long running filters, the :ref:`needs_filter_max_time` configuration option can be used to set a maximum time limit for filter evaluation. +Also see :ref:`needs_uml_process_max_time`, to guard against long running ``needuml`` / ``needarch`` processes containing :ref:`filters `. To debug which filters are being used across your project and their run times, you can enable the :ref:`needs_debug_filters` configuration option. diff --git a/sphinx_needs/config.py b/sphinx_needs/config.py index 37cb85891..0765f7512 100644 --- a/sphinx_needs/config.py +++ b/sphinx_needs/config.py @@ -387,7 +387,11 @@ def __setattr__(self, name: str, value: Any) -> None: filter_max_time: int | float | None = field( default=None, metadata={"rebuild": "html", "types": (type(None), int, float)} ) - """Warn if a filter runs for longer than this time (in seconds).""" + """Warn if process_filter runs for longer than this time (in seconds).""" + uml_process_max_time: int | float | None = field( + default=None, metadata={"rebuild": "html", "types": (type(None), int, float)} + ) + """Warn if process_needuml runs for longer than this time (in seconds).""" flow_engine: Literal["plantuml", "graphviz"] = field( default="plantuml", metadata={"rebuild": "env", "types": (str,)} ) diff --git a/sphinx_needs/data.py b/sphinx_needs/data.py index 440e54544..46d7f0a38 100644 --- a/sphinx_needs/data.py +++ b/sphinx_needs/data.py @@ -656,6 +656,8 @@ class NeedsUmlType(NeedsBaseDataType): is_arch: bool # set in process_needuml content_calculated: str + process_time: float + """Time taken to process the diagram.""" NeedsMutable = NewType("NeedsMutable", Dict[str, NeedsInfoType]) diff --git a/sphinx_needs/directives/needuml.py b/sphinx_needs/directives/needuml.py index 8539cf581..c57b1d0fe 100644 --- a/sphinx_needs/directives/needuml.py +++ b/sphinx_needs/directives/needuml.py @@ -2,6 +2,7 @@ import html import os +import time from typing import TYPE_CHECKING, Any, Dict, List, Sequence, TypedDict from docutils import nodes @@ -16,7 +17,8 @@ from sphinx_needs.diagrams_common import calculate_link from sphinx_needs.directives.needflow._plantuml import make_entity_name from sphinx_needs.filter_common import filter_needs_view -from sphinx_needs.utils import add_doc +from sphinx_needs.logging import log_warning +from sphinx_needs.utils import add_doc, logger if TYPE_CHECKING: from sphinxcontrib.plantuml import plantuml @@ -125,6 +127,7 @@ def run(self) -> Sequence[nodes.Node]: "save": plantuml_code_out_path, "is_arch": is_arch, "content_calculated": "", + "process_time": 0, } add_doc(env, env.docname) @@ -493,11 +496,13 @@ def process_needuml( found_nodes: list[nodes.Element], ) -> None: env = app.env + needs_config = NeedsSphinxConfig(app.config) + uml_data = SphinxNeedsData(env).get_or_create_umls() # for node in doctree.findall(Needuml): for node in found_nodes: id = node.attributes["ids"][0] - current_needuml = SphinxNeedsData(env).get_or_create_umls()[id] + current_needuml = uml_data[id] parent_need_id = None # Check if current needuml is needarch @@ -518,6 +523,7 @@ def process_needuml( [line.strip() for line in config.split("\n") if line.strip()] ) + start = time.perf_counter() puml_node = transform_uml_to_plantuml_node( app=app, uml_content=current_needuml["content"], @@ -526,6 +532,18 @@ def process_needuml( kwargs=current_needuml["extra"], config=config, ) + duration = time.perf_counter() - start + + if ( + needs_config.uml_process_max_time is not None + and duration > needs_config.uml_process_max_time + ): + log_warning( + logger, + f"Uml processing took {duration:.3f}s, which is longer than the configured maximum of {needs_config.uml_process_max_time}s.", + "uml", + location=node, + ) # Add source origin puml_node.line = current_needuml["lineno"] @@ -533,6 +551,7 @@ def process_needuml( # Add calculated needuml content current_needuml["content_calculated"] = puml_node["uml"] + current_needuml["process_time"] = duration try: scale = int(current_needuml["scale"]) diff --git a/tests/test_needarch.py b/tests/test_needarch.py index a6a8467de..053c4a301 100644 --- a/tests/test_needarch.py +++ b/tests/test_needarch.py @@ -1,6 +1,7 @@ from pathlib import Path import pytest +from syrupy.filters import props @pytest.mark.parametrize( @@ -52,7 +53,7 @@ def test_doc_needarch_jinja_import(test_app, snapshot): # check needarch all_needumls = app.env._needs_all_needumls - assert all_needumls == snapshot + assert all_needumls == snapshot(exclude=props("process_time")) @pytest.mark.parametrize( @@ -65,7 +66,7 @@ def test_needarch_jinja_func_need(test_app, snapshot): app.build() all_needumls = app.env._needs_all_needumls - assert all_needumls == snapshot + assert all_needumls == snapshot(exclude=props("process_time")) html = Path(app.outdir, "index.html").read_text(encoding="utf8") assert "as INT_001 [[../index.html#INT_001]]" in html diff --git a/tests/test_needuml.py b/tests/test_needuml.py index 2e8ca0c0b..a0ebfe699 100644 --- a/tests/test_needuml.py +++ b/tests/test_needuml.py @@ -2,6 +2,7 @@ from pathlib import Path import pytest +from syrupy.filters import props from sphinx_needs.data import SphinxNeedsData @@ -17,11 +18,13 @@ def test_doc_build_html(test_app, snapshot): assert Path(app.outdir, "index.html").read_text(encoding="utf8") - all_needs = dict(SphinxNeedsData(app.env).get_needs_view()) + data = SphinxNeedsData(app.env) + + all_needs = dict(data.get_needs_view()) assert all_needs == snapshot() - all_needumls = app.env._needs_all_needumls - assert all_needumls == snapshot + all_needumls = data.get_or_create_umls() + assert all_needumls == snapshot(exclude=props("process_time")) @pytest.mark.parametrize( @@ -170,7 +173,7 @@ def test_needuml_filter(test_app, snapshot): app.build() all_needumls = app.env._needs_all_needumls - assert all_needumls == snapshot + assert all_needumls == snapshot(exclude=props("process_time")) html = Path(app.outdir, "index.html").read_text(encoding="utf8") assert "as ST_002 [[../index.html#ST_002]]" in html @@ -194,7 +197,7 @@ def test_needuml_jinja_func_flow(test_app, snapshot): app.build() all_needumls = app.env._needs_all_needumls - assert all_needumls == snapshot + assert all_needumls == snapshot(exclude=props("process_time")) html = Path(app.outdir, "index.html").read_text(encoding="utf8") assert "as ST_001 [[../index.html#ST_001]]" in html @@ -268,7 +271,7 @@ def test_needuml_jinja_func_ref(test_app, snapshot): app.build() all_needumls = app.env._needs_all_needumls - assert all_needumls == snapshot + assert all_needumls == snapshot(exclude=props("process_time")) html = Path(app.outdir, "index.html").read_text(encoding="utf8") assert "Marvel: [[../index.html#ST_001 Test story]]" in html