Skip to content

Commit

Permalink
♻️ Add needs_filter_max_time/needs_debug_filters, deprecate `expo…
Browse files Browse the repository at this point in the history
…rt_id` (#1309)

Add configuration options to debug and guard against long-running need filters:

- `needs_filter_max_time` warns if any call takes longer than the given time in seconds
- `needs_debug_filters` logs all calls and their run times to `debug_filters.jsonl`

This also deprecates the use of the `export_id` directive option and the writing of filters to the `needs.json`; this information should not be part of a need index and also was not compatible with running the `needs` builder (which skips the write phase)
  • Loading branch information
chrisjsewell authored Oct 2, 2024
1 parent ac4165c commit ec2869f
Show file tree
Hide file tree
Showing 20 changed files with 124 additions and 1,603 deletions.
40 changes: 0 additions & 40 deletions docs/builders.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,6 @@ version(s) inside the **needs.json**.
If you generate and store/archive (e.g. in git) the **needs.json** file
every time you raise your documentation version, you will get a nice history data.

.. _filter_export:

Exporting filters
+++++++++++++++++

.. versionadded:: 0.3.11

The results and filter configuration of a filter based directive, like :ref:`needlist`, :ref:`needtable`
or :ref:`needflow` gets exported, if the option :ref:`export_id` is used in the related directive.

This allows to export specified filter results only.

.. code-block:: rst
.. needtable::
:status: open
:filter: "test" in tags
:export_id: filter_01
.. _needs_builder_format:

Format
Expand All @@ -81,16 +61,6 @@ See also :ref:`needs_json_exclude_fields`, :ref:`needs_json_remove_defaults`, an
"versions": {
"1.0": {
"created": "2017-07-03T11:54:42.433868",
"filters": {
"FILTER_1": {
"amount": 1,
"export_id": "FILTER_1",
"filter": "",
"result": ["IMPL_01"],
"status": [],
"tags": "",
"types": []
},
"needs_schema": {
"$schema": "http://json-schema.org/draft-07/schema#",
"properties": {
Expand Down Expand Up @@ -138,16 +108,6 @@ See also :ref:`needs_json_exclude_fields`, :ref:`needs_json_remove_defaults`, an
},
"1.5": {
"created": "2017-07-03T16:10:31.633425",
"filters": {
"FILTER_1": {
"amount": 1,
"export_id": "FILTER_1",
"filter": "",
"result": ["IMPL_01"],
"status": [],
"tags": "",
"types": []
},
"needs_schema": {
"id": {
"description": "ID of the data.",
Expand Down
2 changes: 1 addition & 1 deletion docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ custom css definitions you need to update them.
------
* Improvement: Added config option :ref:`needs_extra_links` to define additional link types like *blocks*, *tested by* and more.
Supports also style configuration and custom presentation names for links.
* Improvement: Added :ref:`export_id` option for filter directives to export results of filters to ``needs.json``.
* Improvement: Added :ref:`!export_id` option for filter directives to export results of filters to ``needs.json``.
* Improvement: Added config option :ref:`needs_flow_show_links` and related needflow option :ref:`needflow_show_link_names`.
* Improvement: Added config option :ref:`needs_flow_link_types` and related needflow option :ref:`needflow_link_types`.
* Bugfix: Unicode handling for Python 2.7 fixed. (:issue:`86`)
Expand Down
1 change: 1 addition & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@
# -- Options for Needs extension ---------------------------------------

needs_debug_measurement = "READTHEDOCS" in os.environ # run on CI
needs_debug_filters = True

needs_types = [
# Architecture types
Expand Down
19 changes: 19 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,15 @@ If set to False, the filter results contains the original need fields and any ma
needs_allow_unsafe_filters = True
.. _needs_filter_max_time:
needs_filter_max_time
~~~~~~~~~~~~~~~~~~~~~
.. versionadded:: 4.0.0
If set, warn if any :ref:`filter processing <filter>` call takes longer than the given time in seconds.
.. _needs_flow_engine:
needs_flow_engine
Expand Down Expand Up @@ -2177,3 +2186,13 @@ See :ref:`runtime_debugging` for details.
To activate it, set it to ``True``::
needs_debug_measurement = True
.. _needs_debug_filters:
needs_debug_filters
~~~~~~~~~~~~~~~~~~~
.. versionadded:: 4.0.0
If set to ``True``, all calls to :ref:`filter processing <filter>` will be logged to a ``debug_filters.jsonl`` file in the build output directory,
appending a single-line JSON for each filter call.
28 changes: 11 additions & 17 deletions docs/filter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,17 @@ To improve performance, certain common patterns are identified and optimized by
Also filters containing ``and`` will be split into multiple filters and evaluated separately for the above patterns.
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.

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

.. _export_id:
.. deprecated:: 4.0.0

The directive option ``export_id`` was previously used to export filter information in the ``needs.json`` file.
This is deprecated and will be removed in a future version.
Instead use the above :ref:`needs_debug_filters` configuration option.

.. _re_search:

search
Expand All @@ -234,23 +245,6 @@ The second parameter should be one of the above variables(status, id, content, .
.. needlist::
:filter: search("([a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", title)

.. _export_id:

export_id
~~~~~~~~~

.. versionadded:: 0.3.11

If set, the filter results get exported to needs.json, if the builder :ref:`needs_builder` is used::

.. needtable::
:status: open
:filter: "test" in tags
:export_id: filter_01

See :ref:`filter_export` for more details.


.. _filter_code:

Filter code
Expand Down
21 changes: 2 additions & 19 deletions sphinx_needs/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from sphinx_needs.config import NeedsSphinxConfig
from sphinx_needs.data import SphinxNeedsData
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.logging import get_logger
from sphinx_needs.needsfile import NeedsList

LOGGER = get_logger(__name__)
Expand All @@ -25,10 +25,6 @@ class NeedsBuilder(Builder):
where all documents are post-transformed, to improve performance.
It is assumed all need data is already read in the read phase,
and the post-processing of the data is done in the finish phase.
However, if the ``export_id`` option is set for any directives,
the write phase is still run, since this filter data is currently only added there.
A warning is issued in this case.
"""

name = "needs"
Expand All @@ -45,23 +41,14 @@ def write(
updated_docnames: Sequence[str],
method: str = "update",
) -> None:
if not SphinxNeedsData(self.env).has_export_filters:
return
log_warning(
LOGGER,
"At least one use of `export_id` directive option, requires a slower build",
"build",
None,
)
return super().write(build_docnames, updated_docnames, method)
return

def finish(self) -> None:
from sphinx_needs.filter_common import filter_needs_view

data = SphinxNeedsData(self.env)
needs = data.get_needs_view()
needs_config = NeedsSphinxConfig(self.env.config)
filters = data.get_or_create_filters()
version = getattr(self.env.config, "version", "unset")
needs_list = NeedsList(self.env.config, self.outdir, self.srcdir)

Expand Down Expand Up @@ -92,10 +79,6 @@ def finish(self) -> None:
for need in filtered_needs:
needs_list.add_need(version, need)

for need_filter in filters.values():
if need_filter["export_id"]:
needs_list.add_filter(version, need_filter)

try:
needs_list.write_json()
except Exception as e:
Expand Down
16 changes: 13 additions & 3 deletions sphinx_needs/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@ def __setattr__(self, name: str, value: Any) -> None:
default=False, metadata={"rebuild": "html", "types": (bool,)}
)
"""Allow filters to change the need data."""
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)."""
flow_engine: Literal["plantuml", "graphviz"] = field(
default="plantuml", metadata={"rebuild": "env", "types": (str,)}
)
Expand Down Expand Up @@ -525,9 +529,6 @@ def __setattr__(self, name: str, value: Any) -> None:
)
"""Jinja context for rendering templates"""

debug_measurement: bool = field(
default=False, metadata={"rebuild": "html", "types": (bool,)}
)
# add config for needs_id_builder
build_json_per_id: bool = field(
default=False, metadata={"rebuild": "html", "types": (bool,)}
Expand All @@ -536,6 +537,15 @@ def __setattr__(self, name: str, value: Any) -> None:
default="needs_id", metadata={"rebuild": "html", "types": (str,)}
)

debug_measurement: bool = field(
default=False, metadata={"rebuild": "html", "types": (bool,)}
)
"""If True, log runtime information for various functions."""
debug_filters: bool = field(
default=False, metadata={"rebuild": "html", "types": (bool,)}
)
"""If True, log filter processing runtime information."""

@classmethod
def add_config_values(cls, app: Sphinx) -> None:
"""Add all config values to the Sphinx application."""
Expand Down
48 changes: 0 additions & 48 deletions sphinx_needs/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,6 @@
LOGGER = getLogger(__name__)


class NeedsFilterType(TypedDict):
filter: str
"""Filter string."""
status: list[str]
tags: list[str]
types: list[str]
amount: int
export_id: str
"""If set, the filter is exported with this ID in the needs.json file."""
origin: str
"""Origin of the request (e.g. needlist, needtable, needflow)."""
location: str
"""Location of the request (e.g. "docname:lineno")"""
runtime: float
"""Time take to run filter (seconds)."""


class NeedsPartType(TypedDict):
"""Data for a single need part."""

Expand Down Expand Up @@ -529,7 +512,6 @@ class NeedsFilteredBaseType(NeedsBaseDataType):
sort_by: None | str
filter_code: list[str]
filter_func: None | str
export_id: str
filter_warning: str | None
"""If set, the filter is exported with this ID in the needs.json file."""

Expand Down Expand Up @@ -776,29 +758,6 @@ def get_needs_view(self) -> NeedsView:
self.env._needs_view = NeedsView._from_needs(self._env_needs)
return self.env._needs_view

@property
def has_export_filters(self) -> bool:
"""Whether any filters have export IDs."""
try:
return self.env.needs_filters_export_id
except AttributeError:
return False

@has_export_filters.setter
def has_export_filters(self, value: bool) -> None:
self.env.needs_filters_export_id = value

def get_or_create_filters(self) -> dict[str, NeedsFilterType]:
"""Get all filters, mapped by ID.
This is lazily created and cached in the environment.
"""
try:
return self.env.needs_all_filters
except AttributeError:
self.env.needs_all_filters = {}
return self.env.needs_all_filters

def get_or_create_docs(self) -> dict[str, list[str]]:
"""Get mapping of need category to docnames containing the need.
Expand Down Expand Up @@ -897,13 +856,6 @@ def merge_data(
this_data = SphinxNeedsData(env)
other_data = SphinxNeedsData(other)

# update filters
if other_data.has_export_filters:
this_data.has_export_filters = True
# merge these just to be safe,
# although actually all should be added in the write phase
this_data.get_or_create_filters().update(other_data.get_or_create_filters())

# Update needs
needs = this_data._env_needs
other_needs = other_data._env_needs
Expand Down
25 changes: 0 additions & 25 deletions sphinx_needs/debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
from jinja2 import Environment, PackageLoader, select_autoescape
from sphinx.application import Sphinx

from sphinx_needs.data import SphinxNeedsData

TIME_MEASUREMENTS: dict[str, Any] = {} # Stores the timing results
EXECUTE_TIME_MEASUREMENTS = (
False # Will be used to de/activate measurements. Set during a Sphinx Event
Expand Down Expand Up @@ -157,18 +155,6 @@ def _print_timing_results(app: Sphinx) -> None:
print(f' max: {value["max"]:2f}')
print(f' min: {value["min"]:2f} \n')

# print 10 slowest filters
filters = sorted(
SphinxNeedsData(app.env).get_or_create_filters().values(),
key=lambda x: x["runtime"],
reverse=True,
)
if filters:
print("Slowest need filters:")
for filter in filters[:10]:
print(f'{filter["location"]}: {filter["runtime"]:2f}s ({filter["origin"]})')
print("")


def _store_timing_results_json(app: Sphinx, build_data: dict[str, Any]) -> None:
json_result_path = os.path.join(str(app.outdir), "debug_measurement.json")
Expand All @@ -180,16 +166,6 @@ def _store_timing_results_json(app: Sphinx, build_data: dict[str, Any]) -> None:
print(f"Timing measurement results (JSON) stored under {json_result_path}")


def _store_filter_results_json(app: Sphinx) -> None:
json_result_path = os.path.join(str(app.outdir), "debug_filters.json")

data = SphinxNeedsData(app.env).get_or_create_filters()

with open(json_result_path, "w", encoding="utf-8") as f:
json.dump(data, f, indent=4)
print(f"Filter results (JSON) stored under {json_result_path}")


def _store_timing_results_html(app: Sphinx, build_data: dict[str, Any]) -> None:
jinja_env = Environment(
loader=PackageLoader("sphinx_needs"), autoescape=select_autoescape()
Expand All @@ -213,5 +189,4 @@ def process_timing(app: Sphinx, _exception: Exception | None) -> None:

_print_timing_results(app)
_store_timing_results_json(app, build_data)
_store_filter_results_json(app)
_store_timing_results_html(app, build_data)
Loading

0 comments on commit ec2869f

Please sign in to comment.