Skip to content

Commit

Permalink
👌 Add needs_uml_process_max_time configuration (#1314)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chrisjsewell authored Oct 4, 2024
1 parent 250c58f commit 47cc6e1
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 11 deletions.
9 changes: 9 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,15 @@ needs_filter_max_time
If set, warn if any :ref:`filter processing <filter>` 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
Expand Down
1 change: 1 addition & 0 deletions docs/filter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <needuml_jinja_filter>`.

To debug which filters are being used across your project and their run times, you can enable the :ref:`needs_debug_filters` configuration option.

Expand Down
6 changes: 5 additions & 1 deletion sphinx_needs/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,)}
)
Expand Down
2 changes: 2 additions & 0 deletions sphinx_needs/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
23 changes: 21 additions & 2 deletions sphinx_needs/directives/needuml.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import html
import os
import time
from typing import TYPE_CHECKING, Any, Dict, List, Sequence, TypedDict

from docutils import nodes
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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"],
Expand All @@ -526,13 +532,26 @@ 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"]
puml_node.source = env.doc2path(current_needuml["docname"])

# Add calculated needuml content
current_needuml["content_calculated"] = puml_node["uml"]
current_needuml["process_time"] = duration

try:
scale = int(current_needuml["scale"])
Expand Down
5 changes: 3 additions & 2 deletions tests/test_needarch.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from pathlib import Path

import pytest
from syrupy.filters import props


@pytest.mark.parametrize(
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down
15 changes: 9 additions & 6 deletions tests/test_needuml.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from pathlib import Path

import pytest
from syrupy.filters import props

from sphinx_needs.data import SphinxNeedsData

Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 47cc6e1

Please sign in to comment.