Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions doc/eng_sys_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ Starting with [this pr](https://github.com/Azure/azure-sdk-for-python/pull/28345

We default to **enabling** most of our checks like `pylint`, `mypy`, etc. Due to that, most `pyproject.toml` settings will likely be **disabling** checks.

There is also an additional setting to turn on strict sphinx validation, for docstring validation.

Here's an example:

```toml
Expand All @@ -133,7 +131,7 @@ verifytypes = false
pyright = false
pylint = false
black = false
strict_sphinx = false
sphinx = false
```

If a package does not yet have a `pyproject.toml`, creating one with just the section `[tool.azure-sdk-build]` will do no harm to the release of the package in question.
Expand Down Expand Up @@ -216,14 +214,11 @@ Note that the `pylint` environment is configured to run against the **earliest s

### Sphinx and docstring checker

[`Sphinx`](https://www.sphinx-doc.org/en/master/) is the preferred documentation builder for Python libraries. The documentation is always built and attached to each PR builds. Sphinx can be configured to
[`Sphinx`](https://www.sphinx-doc.org/en/master/) is the preferred documentation builder for Python libraries. The documentation is always built and attached to each PR builds. Sphinx is configured to
fail if docstring are invalid, helping to ensure the resulting documentation will be of high quality. Following are the steps to run `sphinx` locally for a specific package with strict docstring checking:

1. Go to root of the package.
2. Make sure the `pyproject.toml` file contains `strict_sphinx = true`
3. Execute following command: `tox run -e sphinx -c ../../../eng/tox/tox.ini --root .`

Note: While as of now the default value is `False`, it will become `True` by mid 2024.
2. Execute following command: `tox run -e sphinx -c ../../../eng/tox/tox.ini --root .`

### Bandit

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,6 @@ extends:
arguments: >-
${{ parameters.BuildTargetingString }}
--service="${{ parameters.ServiceDirectory }}"
--toxenv="strict-sphinx"
--toxenv="sphinx"
env:
GH_TOKEN: $(azuresdk-github-pat)
2 changes: 1 addition & 1 deletion eng/pipelines/templates/stages/python-analyze-weekly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,6 @@ stages:
arguments: >-
${{ parameters.BuildTargetingString }}
--service="${{ parameters.ServiceDirectory }}"
--toxenv="strict-sphinx"
--toxenv="sphinx"
env:
GH_TOKEN: $(azuresdk-github-pat)
22 changes: 5 additions & 17 deletions eng/tox/run_sphinx_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import logging
import sys
from prep_sphinx_env import should_build_docs
from run_sphinx_apidoc import is_mgmt_package
from pkg_resources import Requirement
import ast
import os
Expand All @@ -22,7 +23,7 @@
import shutil

from ci_tools.parsing import ParsedSetup
from ci_tools.functions import get_config_setting


logging.getLogger().setLevel(logging.INFO)

Expand All @@ -40,7 +41,7 @@ def move_output_and_compress(target_dir, package_dir, package_name):
individual_zip_location = os.path.join(ci_doc_dir, package_name, package_name)
shutil.make_archive(individual_zip_location, 'gztar', target_dir)

def sphinx_build(target_dir, output_dir, fail_on_warning=False, package_name=None):
def sphinx_build(target_dir, output_dir, fail_on_warning):
command_array = [
"sphinx-build",
"-b",
Expand All @@ -67,9 +68,6 @@ def sphinx_build(target_dir, output_dir, fail_on_warning=False, package_name=Non
args.working_directory, e.returncode
)
)
if args.strict and in_ci():
from gh_tools.vnext_issue_creator import create_vnext_issue
create_vnext_issue(package_name, "sphinx")
exit(1)

if __name__ == "__main__":
Expand Down Expand Up @@ -108,12 +106,6 @@ def sphinx_build(target_dir, output_dir, fail_on_warning=False, package_name=Non
default=False
)

parser.add_argument(
"--strict",
dest="strict",
default=False
)

args = parser.parse_args()

output_dir = os.path.abspath(args.output_directory)
Expand All @@ -123,20 +115,16 @@ def sphinx_build(target_dir, output_dir, fail_on_warning=False, package_name=Non
pkg_details = ParsedSetup.from_path(package_dir)

if should_build_docs(pkg_details.name):
fail_on_warning = args.strict or get_config_setting(args.package_root, "strict_sphinx", default=False)

# Only data-plane libraries run strict sphinx at the moment
fail_on_warning = not is_mgmt_package(pkg_details.name)
sphinx_build(
target_dir,
output_dir,
fail_on_warning=fail_on_warning,
package_name=pkg_details.name
)

if in_ci() or args.in_ci:
move_output_and_compress(output_dir, package_dir, pkg_details.name)

if args.strict:
from gh_tools.vnext_issue_creator import close_vnext_issue
close_vnext_issue(pkg_details.name, "sphinx")
else:
logging.info("Skipping sphinx build for {}".format(pkg_details.name))
31 changes: 1 addition & 30 deletions eng/tox/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ setenv =
PROXY_URL=http://localhost:5007
deps =
{[base]deps}
sphinx==6.2.1
sphinx==7.3.7
sphinx_rtd_theme==1.3.0
myst_parser==2.0.0
sphinxcontrib-jquery==4.1
Expand All @@ -342,35 +342,6 @@ commands =
-r {tox_root}


[testenv:strict-sphinx]
description="Builds a package's documentation with strict sphinx"
skipsdist = true
skip_install = true
passenv = *
setenv =
{[testenv]setenv}
PROXY_URL=http://localhost:5023
deps =
{[base]deps}
sphinx==7.3.7
sphinx_rtd_theme==1.3.0
myst_parser==2.0.0
sphinxcontrib-jquery==4.1
PyGitHub>=1.59.0
commands =
python {repository_root}/eng/tox/create_package_and_install.py \
-d {envtmpdir}/dist \
-p {tox_root} \
-w {envtmpdir} \
--package-type sdist
python {repository_root}/eng/tox/prep_sphinx_env.py -d {envtmpdir}/dist -t {tox_root}
python {repository_root}/eng/tox/run_sphinx_apidoc.py -w {envtmpdir}/dist -r {tox_root}
python {repository_root}/eng/tox/run_sphinx_build.py \
-w {envtmpdir}/dist/unzipped/docgen \
-o {envtmpdir}/dist/site \
-r {tox_root} \
--strict true

[testenv:depends]
description = Ensures all modules in a target package can be successfully imported
setenv =
Expand Down
59 changes: 19 additions & 40 deletions tools/azure-sdk-tools/gh_tools/vnext_issue_creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

logging.getLogger().setLevel(logging.INFO)

CHECK_TYPE = Literal["mypy", "pylint", "pyright", "sphinx"]
CHECK_TYPE = Literal["mypy", "pylint", "pyright"]


def get_version_running(check_type: CHECK_TYPE) -> str:
Expand Down Expand Up @@ -50,8 +50,6 @@ def get_build_link(check_type: CHECK_TYPE) -> str:
next_id = "c0edaab3-85d6-5e4b-81a8-d1190a6ee92b"
if check_type == "pylint":
next_id = "e1fa7d9e-8471-5a74-cd7d-e1c9a992e07e"
if check_type == "sphinx":
next_id = "0d206e13-b346-5d3f-79e6-d5bfba9e089e"

return f"https://dev.azure.com/azure-sdk/internal/_build/results?buildId={build_id}&view=logs&j={job_id}&t={next_id}"

Expand Down Expand Up @@ -104,43 +102,24 @@ def create_vnext_issue(package_name: str, check_type: CHECK_TYPE) -> None:

version = get_version_running(check_type)
build_link = get_build_link(check_type)
if check_type == "sphinx":
merge_date = "2024-04-15" # This is a one-time event
error_type = "docstring"
guide_link = "[Sphinx and docstring checker](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/eng_sys_checks.md#sphinx-and-docstring-checker)"
title = f"{package_name} needs {error_type} updates for {check_type}"
template = (
f"**ACTION NEEDED:** All {check_type} errors and warnings must be fixed by **{merge_date}**. "
f"The build will begin to fail for this library if errors are not fixed."
f"\n\nThis issue indicates that your library reference documentation is rendering with errors. To avoid customer issues, please ensure you take care of these errors and warnings ASAP."
f"\n\n**Library name:** {package_name}"
f"\n**{check_type.capitalize()} build:** [Link to build ({today.strftime('%Y-%m-%d')})]({build_link})"
f"\n**How to fix:** Run the `strict-{check_type}` tox command at the library package-level and resolve "
f"the {error_type} errors and warnings.\n"
f"1) `../{package_name}>pip install \"tox<5\"`\n"
f"2) `../{package_name}>tox run -e strict-{check_type} -c ../../../eng/tox/tox.ini --root .`\n\n"
f"3) Once resolved, set `strict_sphinx = true` in the package's pyproject.toml file.\n\n"
f"See {guide_link} for more information."
)
else:
merge_date = get_date_for_version_bump(today)
error_type = "linting" if check_type == "pylint" else "typing"
guide_link = "[Pylint Guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/pylint_checking.md)" \
if check_type == "pylint" else "[Typing Guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/static_type_checking.md#run-mypy)"

title = f"{package_name} needs {error_type} updates for {check_type} version {version}"
template = (
f"**ACTION NEEDED:** This version of {check_type} will be merged on **{merge_date}**. "
f"The build will begin to fail for this library if errors are not fixed."
f"\n\n**Library name:** {package_name}"
f"\n**{check_type.capitalize()} version:** {version}"
f"\n**{check_type.capitalize()} errors:** [Link to build ({today.strftime('%Y-%m-%d')})]({build_link})"
f"\n**How to fix:** Run the `next-{check_type}` tox command at the library package-level and resolve "
f"the {error_type} errors.\n"
f"1) `../{package_name}>pip install \"tox<5\"`\n"
f"2) `../{package_name}>tox run -e next-{check_type} -c ../../../eng/tox/tox.ini --root .`\n\n"
f"See the {guide_link} for more information."
)
merge_date = get_date_for_version_bump(today)
error_type = "linting" if check_type == "pylint" else "typing"
guide_link = "[Pylint Guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/pylint_checking.md)" \
if check_type == "pylint" else "[Typing Guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/static_type_checking.md#run-mypy)"

title = f"{package_name} needs {error_type} updates for {check_type} version {version}"
template = (
f"**ACTION NEEDED:** This version of {check_type} will be merged on **{merge_date}**. "
f"The build will begin to fail for this library if errors are not fixed."
f"\n\n**Library name:** {package_name}"
f"\n**{check_type.capitalize()} version:** {version}"
f"\n**{check_type.capitalize()} errors:** [Link to build ({today.strftime('%Y-%m-%d')})]({build_link})"
f"\n**How to fix:** Run the `next-{check_type}` tox command at the library package-level and resolve "
f"the {error_type} errors.\n"
f"1) `../{package_name}>pip install \"tox<5\"`\n"
f"2) `../{package_name}>tox run -e next-{check_type} -c ../../../eng/tox/tox.ini --root .`\n\n"
f"See the {guide_link} for more information."
)

# create an issue for the library failing the vnext check
if not vnext_issue:
Expand Down