make sphinx == strict_sphinx and bump to v7.3 (#35914)

* make sphinx == strict_sphinx and bump to v7.3 * strict sphinx only on dataplane * remove issue creator for sphinx, now mandatory in CI * test that we can still disable if needed * cleaning up docs and weekly template * try disable with artifacts * remove test skip
2024-07-16 14:09:46 -07:00 · 2024-07-16 14:09:46 -07:00 · 9e734665a5
--- a/doc/eng_sys_checks.md
+++ b/doc/eng_sys_checks.md
@ -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
@ -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.
@ -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

--- a/eng/pipelines/templates/stages/python-analyze-weekly-standalone.yml
+++ b/eng/pipelines/templates/stages/python-analyze-weekly-standalone.yml
@ -112,6 +112,6 @@ extends:
                    arguments: >-
                      ${{ parameters.BuildTargetingString }}
                      --service="${{ parameters.ServiceDirectory }}"
-                      --toxenv="strict-sphinx"
+                      --toxenv="sphinx"
                  env:
                    GH_TOKEN: $(azuresdk-github-pat)
--- a/eng/pipelines/templates/stages/python-analyze-weekly.yml
+++ b/eng/pipelines/templates/stages/python-analyze-weekly.yml
@ -109,6 +109,6 @@ stages:
              arguments: >-
                ${{ parameters.BuildTargetingString }}
                --service="${{ parameters.ServiceDirectory }}" 
-                --toxenv="strict-sphinx"
+                --toxenv="sphinx"
            env:
              GH_TOKEN: $(azuresdk-github-pat)
--- a/eng/tox/run_sphinx_build.py
+++ b/eng/tox/run_sphinx_build.py
@ -14,6 +14,7 @@ import os
 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
@ -22,7 +23,7 @@ import io
 import shutil

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

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

@ -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",
@ -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__":
@ -108,12 +106,6 @@ if __name__ == "__main__":
        default=False
    )

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

    output_dir = os.path.abspath(args.output_directory)
@ -123,20 +115,16 @@ if __name__ == "__main__":
    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))
--- a/eng/tox/tox.ini
+++ b/eng/tox/tox.ini
@ -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
@ -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 =
--- a/tools/azure-sdk-tools/gh_tools/vnext_issue_creator.py
+++ b/tools/azure-sdk-tools/gh_tools/vnext_issue_creator.py
@ -18,7 +18,7 @@ from github import Github, Auth

 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:
@ -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}"    

@ -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)"
+    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."
-        )
+    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: