Skip to content

Commit

Permalink
♻️ Extract generate_need from add_need & consolidate warnings (#1318
Browse files Browse the repository at this point in the history
)

This commit makes the code cleaner and easier to understand, when creating needs and handling invalid needs and other build issues, by:

- Extracting `generate_need`: This generates a validated single need dictionary, without adding the need to the build or parsing the content, and then `add_need` calls this.
  Invalid input data results in a `InvalidNeedException`, which replaces/consolidates all the exception/warnings previously raised/emitted.
   Unused exceptions are removed: `NeedsNoIdException`, `NeedsStatusNotAllowed`, `NeedsTagNotAllowed`, `NeedsConstraintNotAllowed`, `NeedsInvalidOption`, `NeedsTemplateException`
- Catch `InvalidNeedException` exceptions in the need/needimport directives and external need loading code, and emit specific warnings, with correct location mapping
- Ensure all warnings have subtypes with certain names and list these in the documentation, explaining how to fail fast and suppress warnings

A bug was also identified, whereby
if the content is not parsed, then `parts` and `arch` need fields will not be populated.
- For importing and loading of external needs, this has been fixed by no longer dropping these keys and allowing them to be passed to `add_need`
- For needs from directives which have been set to `hide`, this is still an open issue
  • Loading branch information
chrisjsewell authored Oct 8, 2024
1 parent 1e473e7 commit 0d5d316
Show file tree
Hide file tree
Showing 43 changed files with 717 additions and 651 deletions.
2 changes: 1 addition & 1 deletion docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Data
----

.. automodule:: sphinx_needs.data
:members: NeedsInfoType, NeedsMutable
:members: NeedsInfoType, NeedsMutable, NeedsPartType

Views
-----
Expand Down
58 changes: 37 additions & 21 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
("py:class", "docutils.statemachine.StringList"),
("py:class", "sphinx_needs.debug.T"),
("py:class", "sphinx_needs.views._LazyIndexes"),
("py:class", "sphinx_needs.config.NeedsSphinxConfig"),
]

rst_epilog = """
Expand Down Expand Up @@ -643,6 +644,7 @@ def custom_defined_func():

# build needs.json to make permalinks work
needs_build_json = True
needs_reproducible_json = True
needs_json_remove_defaults = True

# build needs_json for every needs-id to make detail panel
Expand Down Expand Up @@ -699,18 +701,36 @@ def custom_defined_func():
# needs_report_template = "/needs_templates/report_template.need" # Use custom report template

# -- custom extensions ---------------------------------------
from typing import get_args # noqa: E402

from docutils import nodes # noqa: E402
from docutils.statemachine import StringList # noqa: E402
from sphinx.application import Sphinx # noqa: E402
from sphinx.directives import SphinxDirective # noqa: E402
from sphinx.roles import SphinxRole # noqa: E402

from sphinx_needs.api.need import add_external_need # noqa: E402
from sphinx_needs.api import generate_need # noqa: E402
from sphinx_needs.config import NeedsSphinxConfig # noqa: E402
from sphinx_needs.data import SphinxNeedsData # noqa: E402
from sphinx_needs.logging import ( # noqa: E402
WarningSubTypeDescription,
WarningSubTypes,
)
from sphinx_needs.needsfile import NeedsList # noqa: E402


class NeedsWarningsDirective(SphinxDirective):
"""Directive to list all extension warning subtypes."""

def run(self):
parsed = nodes.container(classes=["needs-warnings"])
content = [
f"- ``needs.{name}`` {WarningSubTypeDescription.get(name, '')}"
for name in get_args(WarningSubTypes)
]
self.state.nested_parse(StringList(content), self.content_offset, parsed)
return [parsed]


class NeedExampleDirective(SphinxDirective):
"""Directive to add example content to the documentation.
Expand Down Expand Up @@ -757,30 +777,26 @@ def create_tutorial_needs(app: Sphinx, _env, _docnames):
We do this dynamically, to avoid having to maintain the JSON file manually.
"""
all_data = SphinxNeedsData(app.env).get_needs_mutable()
needs_config = NeedsSphinxConfig(app.config)
writer = NeedsList(app.config, outdir=app.confdir, confdir=app.confdir)
for i in range(1, 5):
test_id = f"T_00{i}"
# TODO really here we don't want to create the data, without actually adding it to the needs
if test_id in all_data:
data = all_data[test_id]
else:
add_external_need(
app,
"tutorial-test",
id=test_id,
title=f"Unit test {i}",
content=f"Test case {i}",
)
data = all_data.pop(test_id)
writer.add_need(version, data)
# TODO ideally we would only write this file if it has changed (also needimport should add dependency on file)
writer.write_json(
needs_file="tutorial_needs.json", needs_path=str(Path(app.confdir, "_static"))
)
need_item = generate_need(
needs_config,
need_type="tutorial-test",
id=f"T_00{i}",
title=f"Unit test {i}",
content=f"Test case {i}",
)
writer.add_need(version, need_item)
json_str = writer.dump_json()
outpath = Path(app.confdir, "_static", "tutorial_needs.json")
if outpath.is_file() and outpath.read_text() == json_str:
return # only write this file if it has changed
outpath.write_text(json_str)


def setup(app: Sphinx):
app.add_directive("need-example", NeedExampleDirective)
app.add_directive("need-warnings", NeedsWarningsDirective)
app.add_role("need_config_default", NeedConfigDefaultRole())
app.connect("env-before-read-docs", create_tutorial_needs, priority=600)
20 changes: 18 additions & 2 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@ Add **sphinx_needs** to your extensions.
extensions = ["sphinx_needs",]
.. _config-warnings:

Build Warnings
--------------

sphinx-needs is designed to be durable and only except when absolutely necessary.
Any non-fatal issues during the build are logged as Sphinx warnings.

If you wish to "fail-fast" during a build, see the `--fail-on-warning <https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-W>`__ command-line option
(in sphinx 8.1 ``--exception-on-warning``).

You can also use this in conjunction with the `suppress_warnings <https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-suppress_warnings>`__ configuration option,
to suppress specific warnings.

Find below a list of all warnings, which can be suppressed:

.. need-warnings::

.. _inc_build:

Incremental build support
Expand Down Expand Up @@ -855,8 +873,6 @@ So no ID is autogenerated any more, if this option is set to True:
By default this option is set to **False**.
If an ID is missing Sphinx throws the exception "NeedsNoIdException" and stops the build.
**Example**:
.. code-block:: rst
Expand Down
6 changes: 4 additions & 2 deletions sphinx_needs/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@
add_need_type,
get_need_types,
)
from .need import add_external_need, add_need, del_need, get_needs_view, make_hashed_id
from .exceptions import InvalidNeedException
from .need import add_external_need, add_need, del_need, generate_need, get_needs_view

__all__ = (
"add_dynamic_function",
"add_extra_option",
"add_external_need",
"add_need",
"InvalidNeedException",
"add_need_type",
"del_need",
"generate_need",
"get_need_types",
"get_needs_view",
"make_hashed_id",
)
71 changes: 48 additions & 23 deletions sphinx_needs/api/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import annotations

from typing import Literal

from sphinx.errors import SphinxError, SphinxWarning


Expand All @@ -11,41 +13,64 @@ class NeedsApiConfigException(SphinxError):
"""


class NeedsApiConfigWarning(SphinxWarning):
pass


class NeedsNoIdException(SphinxError):
pass

class InvalidNeedException(Exception):
"""Raised when a need could not be created/added, due to a validation issue."""

def __init__(
self,
type_: Literal[
"invalid_kwargs",
"invalid_type",
"missing_id",
"invalid_id",
"duplicate_id",
"invalid_status",
"invalid_tags",
"invalid_constraints",
"invalid_jinja_content",
"invalid_template",
"global_option",
],
message: str,
) -> None:
self._type = type_
self._message = message
super().__init__(f"{message} [{type_}]")

@property
def type(
self,
) -> Literal[
"invalid_kwargs",
"invalid_type",
"missing_id",
"invalid_id",
"duplicate_id",
"invalid_status",
"invalid_tags",
"invalid_constraints",
"invalid_jinja_content",
"invalid_template",
"global_option",
]:
return self._type

@property
def message(self) -> str:
return self._message

class NeedsStatusNotAllowed(SphinxError):
pass


class NeedsTagNotAllowed(SphinxError):
class NeedsApiConfigWarning(SphinxWarning):
pass


class NeedsInvalidException(SphinxError):
pass


class NeedsConstraintNotAllowed(SphinxError):
pass


class NeedsConstraintFailed(SphinxError):
pass


class NeedsInvalidOption(SphinxError):
pass


class NeedsTemplateException(SphinxError):
pass


class NeedsInvalidFilter(SphinxError):
pass
Loading

0 comments on commit 0d5d316

Please sign in to comment.