Skip to content

Commit

Permalink
🔧 Centralise warning logging (sphinx 8 compat) (#1236)
Browse files Browse the repository at this point in the history
Currently, we prepend the warning type/subtype to the end of each warning message,
but in sphinx 7.3 I added this capability directly in sphinx core (under the `show_warning_types` config),
and in sphinx 8.0 I changed the default to `show_warning_types=True` (sphinx-doc/sphinx#12597)

Therefore, for newer versions of sphinx, we no longer need to do this,
and thus centralising the warning logging allows for us to add a check for this and prepend only if necessary.
Additionally, it is also a good way to ensure all warnings are given the `needs` type
  • Loading branch information
chrisjsewell authored Aug 26, 2024
1 parent 28dc088 commit c04dc4e
Show file tree
Hide file tree
Showing 27 changed files with 278 additions and 203 deletions.
44 changes: 23 additions & 21 deletions sphinx_needs/api/need.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from sphinx_needs.data import NeedsInfoType, SphinxNeedsData
from sphinx_needs.directives.needuml import Needuml, NeedumlException
from sphinx_needs.filter_common import filter_single_need
from sphinx_needs.logging import get_logger
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.nodes import Need
from sphinx_needs.roles.need_part import find_parts, update_need_with_parts
from sphinx_needs.utils import jinja_parse
Expand Down Expand Up @@ -167,11 +167,11 @@ def run():
# Log messages for need elements that could not be imported.
configured_need_types = [ntype["directive"] for ntype in types]
if need_type not in configured_need_types:
logger.warning(
log_warning(
logger,
f"Couldn't create need {id}. Reason: The need-type (i.e. `{need_type}`) is not set "
"in the project's 'need_types' configuration in conf.py. [needs.add]",
type="needs",
subtype="add",
"in the project's 'need_types' configuration in conf.py.",
"add",
location=(docname, lineno) if docname else None,
)

Expand Down Expand Up @@ -243,11 +243,11 @@ def run():
new_tags = [] # Shall contain only valid tags
for i in range(len(tags)):
if len(tags[i]) == 0 or tags[i].isspace():
logger.warning(
log_warning(
logger,
f"Scruffy tag definition found in need {need_id!r}. "
"Defined tag contains spaces only. [needs.add]",
type="needs",
subtype="add",
"Defined tag contains spaces only.",
"add",
location=(docname, lineno) if docname else None,
)
else:
Expand Down Expand Up @@ -282,11 +282,11 @@ def run():
new_constraints = [] # Shall contain only valid constraints
for i in range(len(constraints)):
if len(constraints[i]) == 0 or constraints[i].isspace():
logger.warning(
log_warning(
logger,
f"Scruffy constraint definition found in need {need_id!r}. "
"Defined constraint contains spaces only. [needs.add]",
type="needs",
subtype="add",
"Defined constraint contains spaces only.",
"add",
location=(docname, lineno) if docname else None,
)
else:
Expand Down Expand Up @@ -325,10 +325,10 @@ def run():
"from the content, then ensure the first sentence of the "
"requirements are different."
)
logger.warning(
message + " [needs.duplicate_id]",
type="needs",
subtype="duplicate_id",
log_warning(
logger,
message,
"duplicate_id",
location=(docname, lineno) if docname else None,
)
return []
Expand Down Expand Up @@ -613,7 +613,7 @@ def del_need(app: Sphinx, need_id: str) -> None:
if need_id in needs:
del needs[need_id]
else:
logger.warning(f"Given need id {need_id} not exists! [needs]", type="needs")
log_warning(logger, f"Given need id {need_id} not exists!", None, None)


def add_external_need(
Expand Down Expand Up @@ -715,10 +715,12 @@ def _read_in_links(links_string: None | str | list[str]) -> list[str]:
link_list = links_string
for link in link_list:
if link.isspace():
logger.warning(
log_warning(
logger,
f"Grubby link definition found in need {id}. "
"Defined link contains spaces only. [needs]",
type="needs",
"Defined link contains spaces only.",
None,
None,
)
else:
links.append(link.strip())
Expand Down
9 changes: 5 additions & 4 deletions sphinx_needs/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from sphinx_needs.config import NeedsSphinxConfig
from sphinx_needs.data import NeedsInfoType, SphinxNeedsData
from sphinx_needs.directives.need import post_process_needs_data
from sphinx_needs.logging import get_logger
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.needsfile import NeedsList

LOGGER = get_logger(__name__)
Expand Down Expand Up @@ -49,10 +49,11 @@ def write(
) -> None:
if not SphinxNeedsData(self.env).has_export_filters:
return
LOGGER.warning(
log_warning(
LOGGER,
"At least one use of `export_id` directive option, requires a slower build",
type="needs",
subtype="build",
"build",
None,
)
return super().write(build_docnames, updated_docnames, method)

Expand Down
10 changes: 6 additions & 4 deletions sphinx_needs/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

from sphinx.util.logging import getLogger

from sphinx_needs.logging import log_warning

if TYPE_CHECKING:
from docutils.nodes import Element, Text
from sphinx.application import Sphinx
Expand Down Expand Up @@ -784,10 +786,10 @@ def merge_data(
f"A need with ID {other_id} already exists, "
f"title: {other_need['title']!r}."
)
LOGGER.warning(
message + " [needs.duplicate_id]",
type="needs",
subtype="duplicate_id",
log_warning(
LOGGER,
message,
"duplicate_id",
location=(_docname, other_need["lineno"]) if _docname else None,
)
else:
Expand Down
33 changes: 18 additions & 15 deletions sphinx_needs/directives/need.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
)
from sphinx_needs.functions.functions import check_and_get_content
from sphinx_needs.layout import build_need
from sphinx_needs.logging import get_logger
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.need_constraints import process_constraints
from sphinx_needs.nodes import Need
from sphinx_needs.utils import (
Expand Down Expand Up @@ -153,10 +153,11 @@ def read_in_links(self, name: str) -> list[str]:
if links_string:
for link in re.split(r";|,", links_string):
if link.isspace():
LOGGER.warning(
log_warning(
LOGGER,
f"Grubby link definition found in need '{self.trimmed_title}'. "
"Defined link contains spaces only. [needs]",
type="needs",
"Defined link contains spaces only.",
None,
location=(self.env.docname, self.lineno),
)
else:
Expand Down Expand Up @@ -210,10 +211,11 @@ def _get_full_title(self) -> str:
`:title_from_content:` was set, and '' if no title is to be derived)."""
if len(self.arguments) > 0: # a title was passed
if "title_from_content" in self.options:
self.log.warning(
log_warning(
self.log,
f'need "{self.arguments[0]}" has :title_from_content: set, '
f"but a title was provided. (see file {self.docname}) [needs]",
type="needs",
f"but a title was provided. (see file {self.docname})",
None,
location=(self.env.docname, self.lineno),
)
return self.arguments[0] # type: ignore[no-any-return]
Expand Down Expand Up @@ -471,17 +473,18 @@ def check_links(needs: dict[str, NeedsInfoType], config: NeedsSphinxConfig) -> N
# we want to provide that URL as the location of the warning,
# otherwise we use the location of the need in the source file
if need.get("is_external", False):
LOGGER.warning(
f"{need['external_url']}: {message} [needs.external_link_outgoing]",
type="needs",
subtype="external_link_outgoing",
log_warning(
LOGGER,
f"{need['external_url']}: {message}",
"external_link_outgoing",
None,
)
else:
LOGGER.warning(
f"{message} [needs.link_outgoing]",
log_warning(
LOGGER,
message,
"link_outgoing",
location=(need["docname"], need["lineno"]),
type="needs",
subtype="link_outgoing",
)


Expand Down
11 changes: 6 additions & 5 deletions sphinx_needs/directives/needbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from sphinx_needs.config import NeedsSphinxConfig
from sphinx_needs.data import NeedsBarType, SphinxNeedsData
from sphinx_needs.filter_common import FilterBase, filter_needs, prepare_need_list
from sphinx_needs.logging import get_logger
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.utils import (
add_doc,
import_matplotlib,
Expand Down Expand Up @@ -185,12 +185,13 @@ def process_needbar(
matplotlib = import_matplotlib()

if matplotlib is None and found_nodes and needs_config.include_needs:
logger.warning(
log_warning(
logger,
"Matplotlib is not installed and required by needbar. "
"Install with `sphinx-needs[plotting]` to use. [needs.mpl]",
"Install with `sphinx-needs[plotting]` to use.",
"mpl",
None,
once=True,
type="needs",
subtype="mpl",
)

# NEEDFLOW
Expand Down
32 changes: 17 additions & 15 deletions sphinx_needs/directives/needextend.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from sphinx_needs.config import NeedsSphinxConfig
from sphinx_needs.data import NeedsExtendType, NeedsInfoType, SphinxNeedsData
from sphinx_needs.filter_common import filter_needs
from sphinx_needs.logging import get_logger
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.utils import add_doc

logger = get_logger(__name__)
Expand Down Expand Up @@ -112,14 +112,14 @@ def extend_needs_data(
needs_config.id_regex, need_filter
):
# an unknown ID
error = f"Provided id {need_filter!r} for needextend does not exist. [needs.extend]"
error = f"Provided id {need_filter!r} for needextend does not exist."
if current_needextend["strict"]:
raise NeedsInvalidFilter(error)
else:
logger.warning(
log_warning(
logger,
error,
type="needs",
subtype="extend",
"extend",
location=(
current_needextend["docname"],
current_needextend["lineno"],
Expand All @@ -139,10 +139,10 @@ def extend_needs_data(
),
)
except NeedsInvalidFilter as e:
logger.warning(
f"Invalid filter {need_filter!r}: {e} [needs.extend]",
type="needs",
subtype="extend",
log_warning(
logger,
f"Invalid filter {need_filter!r}: {e}",
"extend",
location=(
current_needextend["docname"],
current_needextend["lineno"],
Expand All @@ -162,9 +162,10 @@ def extend_needs_data(
if option_name in link_names:
for item, is_function in _split_value(value):
if (not is_function) and (item not in all_needs):
logger.warning(
f"Provided link id {item} for needextend does not exist. [needs]",
type="needs",
log_warning(
logger,
f"Provided link id {item} for needextend does not exist.",
None,
location=(
current_needextend["docname"],
current_needextend["lineno"],
Expand Down Expand Up @@ -196,9 +197,10 @@ def extend_needs_data(
need[option] = []
for item, is_function in _split_value(value):
if (not is_function) and (item not in all_needs):
logger.warning(
f"Provided link id {item} for needextend does not exist. [needs]",
type="needs",
log_warning(
logger,
f"Provided link id {item} for needextend does not exist.",
None,
location=(
current_needextend["docname"],
current_needextend["lineno"],
Expand Down
10 changes: 6 additions & 4 deletions sphinx_needs/directives/needflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from sphinx_needs.diagrams_common import calculate_link, create_legend
from sphinx_needs.directives.utils import no_needs_found_paragraph
from sphinx_needs.filter_common import FilterBase, filter_single_need, process_filters
from sphinx_needs.logging import get_logger
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.utils import (
add_doc,
get_scale,
Expand Down Expand Up @@ -377,13 +377,15 @@ def process_needflow(
option_link_types = [link.upper() for link in current_needflow["link_types"]]
for lt in option_link_types:
if lt not in link_type_names:
logger.warning(
"Unknown link type {link_type} in needflow {flow}. Allowed values: {link_types} [needs]".format(
log_warning(
logger,
"Unknown link type {link_type} in needflow {flow}. Allowed values: {link_types}".format(
link_type=lt,
flow=current_needflow["target_id"],
link_types=",".join(link_type_names),
),
type="needs",
None,
None,
)

# compute the allowed link names
Expand Down
10 changes: 5 additions & 5 deletions sphinx_needs/directives/needgantt.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
no_needs_found_paragraph,
)
from sphinx_needs.filter_common import FilterBase, filter_single_need, process_filters
from sphinx_needs.logging import get_logger
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.utils import MONTH_NAMES, add_doc, remove_node_from_tree

logger = get_logger(__name__)
Expand Down Expand Up @@ -250,11 +250,11 @@ def process_needgantt(
if need["docname"]
else ""
)
logger.warning(
log_warning(
logger,
"Duration not set or invalid for needgantt chart. "
f"Need: {need['id']!r}{need_location}. Duration: {duration!r} [needs.gantt]",
type="needs",
subtype="gantt",
f"Need: {need['id']!r}{need_location}. Duration: {duration!r}",
"gantt",
location=node,
)
duration = 1
Expand Down
15 changes: 9 additions & 6 deletions sphinx_needs/directives/needimport.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from sphinx_needs.debug import measure_time
from sphinx_needs.defaults import string_to_boolean
from sphinx_needs.filter_common import filter_single_need
from sphinx_needs.logging import log_warning
from sphinx_needs.needsfile import check_needs_file
from sphinx_needs.utils import add_doc, logger

Expand Down Expand Up @@ -95,11 +96,12 @@ def run(self) -> Sequence[nodes.Node]:
)
if os.path.exists(old_need_import_path):
correct_need_import_path = old_need_import_path
logger.warning(
log_warning(
logger,
"Deprecation warning: Relative path must be relative to the current document in future, "
"not to the conf.py location. Use a starting '/', like '/needs.json', to make the path "
"relative to conf.py. [needs]",
type="needs",
"relative to conf.py.",
None,
location=(self.env.docname, self.lineno),
)
else:
Expand Down Expand Up @@ -172,9 +174,10 @@ def run(self) -> Sequence[nodes.Node]:
if filter_single_need(filter_context, needs_config, filter_string):
needs_list_filtered[key] = need
except Exception as e:
logger.warning(
f"needimport: Filter {filter_string} not valid. Error: {e}. {self.docname}{self.lineno} [needs]",
type="needs",
log_warning(
logger,
f"needimport: Filter {filter_string} not valid. Error: {e}. {self.docname}{self.lineno}",
None,
location=(self.env.docname, self.lineno),
)

Expand Down
Loading

0 comments on commit c04dc4e

Please sign in to comment.