diff --git a/doc/eng_sys_checks.md b/doc/eng_sys_checks.md index 42abc6de442f..e89dab76fcb8 100644 --- a/doc/eng_sys_checks.md +++ b/doc/eng_sys_checks.md @@ -112,6 +112,8 @@ This is the most useful skip, but the following skip variables are also supporte - Omit checking that a package's dependencies are on PyPI before releasing. - `Skip.KeywordCheck` - Omit checking that a package's keywords are correctly formulated before releasing. +- `Skip.Black` + - Omit checking `black` in the `analyze` job. ## The pyproject.toml @@ -172,7 +174,7 @@ You can enable test logging in a pipeline by setting the queue time variable `PY `PYTEST_LOG_LEVEL=INFO` -This also works locally with tox by setting the `PYTEST_LOG_LEVEL` environment variable. +This also works locally with tox by setting the `PYTEST_LOG_LEVEL` environment variable. Note that if you want DEBUG level logging with sensitive information unredacted in the test logs, then you still must pass `logging_enable=True` into the client(s) being used in tests. @@ -237,17 +239,17 @@ fail if docstring are invalid, helping to ensure the resulting documentation wil #### Opt-in to formatting validation -Make the following change to your projects `ci.yml`: +Ensure that `black = true` is present within your `pyproject.toml`: ```yml -extends: - template: ../../eng/pipelines/templates/stages/archetype-sdk-client.yml - parameters: - ... - ValidateFormatting: true - ... +[tool.azure-sdk-build] +...other checks enabled/disabled +black = true +...other checks enabled/disabled ``` +to opt into the black invocation. + #### Running locally 1. Go to package root directory. diff --git a/eng/pipelines/templates/jobs/ci.yml b/eng/pipelines/templates/jobs/ci.yml index 7e587a2e8762..d4af786175bd 100644 --- a/eng/pipelines/templates/jobs/ci.yml +++ b/eng/pipelines/templates/jobs/ci.yml @@ -49,9 +49,6 @@ parameters: - name: VerifyAutorest type: boolean default: false - - name: ValidateFormatting - type: boolean - default: false - name: UnsupportedToxEnvironments type: string default: '' @@ -229,7 +226,6 @@ jobs: TestPipeline: ${{ parameters.TestPipeline }} Artifacts: ${{ parameters.Artifacts }} VerifyAutorest: ${{ parameters.VerifyAutorest }} - ValidateFormatting: ${{ parameters.ValidateFormatting }} GenerateApiReviewForManualOnly: ${{ parameters.GenerateApiReviewForManualOnly }} - template: /eng/common/pipelines/templates/jobs/generate-job-matrix.yml diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index 12574fd33a96..f87bfd1b80e9 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -63,9 +63,6 @@ parameters: - name: VerifyAutorest type: boolean default: false - - name: ValidateFormatting - type: boolean - default: false - name: TestProxy type: boolean default: true @@ -107,7 +104,6 @@ extends: MatrixFilters: ${{ parameters.MatrixFilters }} MatrixReplace: ${{ parameters.MatrixReplace }} VerifyAutorest: ${{ parameters.VerifyAutorest }} - ValidateFormatting: ${{ parameters.ValidateFormatting }} TestProxy: ${{ parameters.TestProxy }} GenerateApiReviewForManualOnly: ${{ parameters.GenerateApiReviewForManualOnly }} diff --git a/eng/pipelines/templates/steps/analyze.yml b/eng/pipelines/templates/steps/analyze.yml index 38a61b2dac90..f87cc359ef3b 100644 --- a/eng/pipelines/templates/steps/analyze.yml +++ b/eng/pipelines/templates/steps/analyze.yml @@ -5,7 +5,6 @@ parameters: Artifacts: [] TestPipeline: false VerifyAutorest: false - ValidateFormatting: false GenerateApiReviewForManualOnly: false # Please use `$(TargetingString)` to refer to the python packages glob string. This variable is set from resolve-package-targeting.yml. @@ -110,11 +109,9 @@ steps: ServiceDirectory: ${{ parameters.ServiceDirectory }} AdditionalTestArgs: ${{ parameters.AdditionalTestArgs }} - - ${{ if parameters.ValidateFormatting }}: - - template: run_black.yml - parameters: - ServiceDirectory: ${{ parameters.ServiceDirectory }} - ValidateFormatting: ${{ parameters.ValidateFormatting }} + - template: run_black.yml + parameters: + ServiceDirectory: ${{ parameters.ServiceDirectory }} - task: PythonScript@0 displayName: 'Run Keyword Validation Check' diff --git a/eng/pipelines/templates/steps/run_black.yml b/eng/pipelines/templates/steps/run_black.yml index 4828c72c45a1..7bc123fe9e45 100644 --- a/eng/pipelines/templates/steps/run_black.yml +++ b/eng/pipelines/templates/steps/run_black.yml @@ -1,6 +1,5 @@ parameters: ServiceDirectory: '' - ValidateFormatting: false EnvVars: {} AdditionalTestArgs: '' @@ -19,10 +18,13 @@ steps: - task: PythonScript@0 displayName: 'Run Black' inputs: - scriptPath: 'scripts/devops_tasks/validate_formatting.py' + scriptPath: 'scripts/devops_tasks/dispatch_tox.py' arguments: >- "$(TargetingString)" - --service_directory="${{ parameters.ServiceDirectory }}" - --validate="${{ parameters.ValidateFormatting }}" + --mark_arg="${{ parameters.TestMarkArgument }}" + --service="${{ parameters.ServiceDirectory }}" + --toxenv="black" + --filter-type="Omit_management" + ${{ parameters.AdditionalTestArgs }} env: ${{ parameters.EnvVars }} - condition: and(succeededOrFailed(), ne(variables['Skip.Pylint'],'true')) + condition: and(succeededOrFailed(), ne(variables['Skip.Black'],'true')) \ No newline at end of file diff --git a/eng/tox/run_black.py b/eng/tox/run_black.py new file mode 100644 index 000000000000..6a8f0075f563 --- /dev/null +++ b/eng/tox/run_black.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python + +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +# This script is used to execute pylint within a tox environment. Depending on which package is being executed against, +# a failure may be suppressed. + +import subprocess +import argparse +import os +import logging +import sys + +from ci_tools.environment_exclusions import is_check_enabled +from ci_tools.parsing import ParsedSetup +from ci_tools.variables import in_ci + +logging.getLogger().setLevel(logging.INFO) + +root_dir = os.path.abspath(os.path.join(os.path.abspath(__file__), "..", "..", "..")) + +if __name__ == "__main__": + parser = argparse.ArgumentParser( + description="Run black against target folder. Uses the black config in eng/black-pyproject.toml." + ) + + parser.add_argument( + "-t", + "--target", + dest="target_package", + help="The target package directory on disk.", + required=True, + ) + + args = parser.parse_args() + + pkg_dir = os.path.abspath(args.target_package) + pkg_details = ParsedSetup.from_path(pkg_dir) + configFileLocation = os.path.join(root_dir, "eng/black-pyproject.toml") + + if in_ci(): + if not is_check_enabled(args.target_package, "black"): + logging.info( + f"Package {pkg_details.name} opts-out of black check." + ) + exit(0) + + try: + run_result = subprocess.run( + [ + sys.executable, + "-m", + "black", + f"--config={configFileLocation}", + args.target_package, + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE + ) + + if run_result.stderr and "reformatted" in run_result.stderr.decode("utf-8"): + if in_ci(): + logging.info(f"The package {pkg_details.name} needs reformat. Run the `black` tox env locally to reformat.") + exit(1) + else: + logging.info(f"The package {pkg_details.name} was reformatted.") + + except subprocess.CalledProcessError as e: + logging.error( + f"Unable to invoke black for {pkg_details.name}. Ran into exception {e}." + ) + diff --git a/eng/tox/tox.ini b/eng/tox/tox.ini index 872023d6fb89..6f0676fb688d 100644 --- a/eng/tox/tox.ini +++ b/eng/tox/tox.ini @@ -531,9 +531,9 @@ description=Runs the code formatter black skip_install=true deps= black==24.4.0 + -rdev_requirements.txt commands= - black --config {repository_root}/eng/black-pyproject.toml {posargs} - + python {repository_root}/eng/tox/run_black.py -t {tox_root} [testenv:generate] description=Regenerate the code diff --git a/scripts/devops_tasks/validate_formatting.py b/scripts/devops_tasks/validate_formatting.py deleted file mode 100644 index 5e9dc1614f83..000000000000 --- a/scripts/devops_tasks/validate_formatting.py +++ /dev/null @@ -1,107 +0,0 @@ -#!/usr/bin/env python - -# -------------------------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for license information. -# -------------------------------------------------------------------------------------------- - -import argparse -import os -import logging -import sys -import subprocess - -logging.getLogger().setLevel(logging.INFO) -from ci_tools.functions import discover_targeted_packages -from ci_tools.environment_exclusions import is_check_enabled - -root_dir = os.path.abspath(os.path.join(os.path.abspath(__file__), "..", "..", "..")) -sdk_dir = os.path.join(root_dir, "sdk") - - -def run_black(glob_string, service_dir): - results = [] - logging.info("Running black for {}".format(service_dir)) - - if service_dir and service_dir != "auto": - target_dir = os.path.join(root_dir, "sdk", service_dir) - else: - target_dir = root_dir - - discovered_packages = discover_targeted_packages(glob_string, target_dir) - - for package in discovered_packages: - package_name = os.path.basename(package) - - if is_check_enabled(package, "black", True): - out = subprocess.Popen( - [ - "tox", - "-qqq", - "--conf", - os.path.join(root_dir, "eng", "tox", "tox.ini"), - "--root", - root_dir, - "run", - "-e", - "black", - "--", - package, - ], - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - cwd=root_dir, - ) - - stdout, stderr = out.communicate() - - if stderr: - results.append((package_name, stderr)) - - if out.returncode > 0: - print( - f"black ran into an unexpected failure while analyzing the code for {package_name}" - ) - if stdout: - print(stdout.decode("utf-8")) - exit(out.returncode) - - if stdout and "reformatted" in stdout.decode("utf-8"): - results.append((package_name, False)) - else: - print(f"black succeeded against {package_name}") - - return results - - -if __name__ == "__main__": - parser = argparse.ArgumentParser(description="Run black to verify formatted code.") - - parser.add_argument( - "glob_string", - nargs="?", - help=( - "A comma separated list of glob strings that will target the top level directories that contain packages." - 'Examples: All = "azure*", Single = "azure-keyvault", Targeted Multiple = "azure-keyvault,azure-mgmt-resource"' - ), - ) - parser.add_argument( - "--service_directory", help="Directory of the package being tested" - ) - - parser.add_argument("--validate", help=("Flag that enables formatting validation.")) - - args = parser.parse_args() - if args.validate != "False": - results = run_black(args.glob_string, args.service_directory) - - if len(results) > 0: - for result in results: - error = "Code needs reformat." if result[1] == False else error - logging.error(f"Black run for {result[0]} ran into an issue: {error}") - - raise ValueError( - "Found difference between formatted code and current commit. Check https://aka.ms/azsdk/python/black for how to fix it." - ) - else: - print("Skipping formatting validation") diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/pyproject.toml b/sdk/appconfiguration/azure-appconfiguration-provider/pyproject.toml index e00361912969..2a482558eb95 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/pyproject.toml +++ b/sdk/appconfiguration/azure-appconfiguration-provider/pyproject.toml @@ -1,2 +1,3 @@ [tool.azure-sdk-build] pyright = false +black = true \ No newline at end of file diff --git a/sdk/appconfiguration/azure-appconfiguration/pyproject.toml b/sdk/appconfiguration/azure-appconfiguration/pyproject.toml index e00361912969..2a482558eb95 100644 --- a/sdk/appconfiguration/azure-appconfiguration/pyproject.toml +++ b/sdk/appconfiguration/azure-appconfiguration/pyproject.toml @@ -1,2 +1,3 @@ [tool.azure-sdk-build] pyright = false +black = true \ No newline at end of file diff --git a/sdk/appconfiguration/ci.yml b/sdk/appconfiguration/ci.yml index ef0994da80bd..e2daba2265ac 100644 --- a/sdk/appconfiguration/ci.yml +++ b/sdk/appconfiguration/ci.yml @@ -31,7 +31,6 @@ extends: TestProxy: true ServiceDirectory: appconfiguration VerifyAutorest: false - ValidateFormatting: true Artifacts: - name: azure-appconfiguration safeName: azureappconfiguration diff --git a/sdk/containerregistry/azure-containerregistry/pyproject.toml b/sdk/containerregistry/azure-containerregistry/pyproject.toml index e00361912969..2a482558eb95 100644 --- a/sdk/containerregistry/azure-containerregistry/pyproject.toml +++ b/sdk/containerregistry/azure-containerregistry/pyproject.toml @@ -1,2 +1,3 @@ [tool.azure-sdk-build] pyright = false +black = true \ No newline at end of file diff --git a/sdk/containerregistry/azure-mgmt-containerregistry/pyproject.toml b/sdk/containerregistry/azure-mgmt-containerregistry/pyproject.toml index 42a7f73e0386..9151f46eea2b 100644 --- a/sdk/containerregistry/azure-mgmt-containerregistry/pyproject.toml +++ b/sdk/containerregistry/azure-mgmt-containerregistry/pyproject.toml @@ -1,2 +1,3 @@ [tool.azure-sdk-build] breaking = false +black = true \ No newline at end of file diff --git a/sdk/containerregistry/ci.yml b/sdk/containerregistry/ci.yml index b120c694209b..5588152d6182 100644 --- a/sdk/containerregistry/ci.yml +++ b/sdk/containerregistry/ci.yml @@ -30,7 +30,6 @@ extends: parameters: TestProxy: true ServiceDirectory: containerregistry - ValidateFormatting: true Artifacts: - name: azure-mgmt-containerregistry safeName: azuremgmtcontainerregistry diff --git a/sdk/core/azure-core-experimental/pyproject.toml b/sdk/core/azure-core-experimental/pyproject.toml index b2797a3242f2..42684a11ae1b 100644 --- a/sdk/core/azure-core-experimental/pyproject.toml +++ b/sdk/core/azure-core-experimental/pyproject.toml @@ -2,3 +2,4 @@ type_check_samples = false pyright = false mypy = true +black = true \ No newline at end of file diff --git a/sdk/core/azure-core-tracing-opentelemetry/pyproject.toml b/sdk/core/azure-core-tracing-opentelemetry/pyproject.toml index f9fdc0526317..cf19e90d6267 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/pyproject.toml +++ b/sdk/core/azure-core-tracing-opentelemetry/pyproject.toml @@ -1,3 +1,4 @@ [tool.azure-sdk-build] type_check_samples = false pyright = false +black = true \ No newline at end of file diff --git a/sdk/core/azure-core/pyproject.toml b/sdk/core/azure-core/pyproject.toml index 20f30ce7f59b..c847603044f5 100644 --- a/sdk/core/azure-core/pyproject.toml +++ b/sdk/core/azure-core/pyproject.toml @@ -3,6 +3,7 @@ mypy = true type_check_samples = true verifytypes = true pyright = false +black = true # For test environments or static checks where a check should be run by default, not explicitly disabling will enable the check. # pylint is enabled by default, so there is no reason for a pylint = true in every pyproject.toml. # diff --git a/sdk/core/azure-mgmt-core/pyproject.toml b/sdk/core/azure-mgmt-core/pyproject.toml index e17295dc5507..5598dd050a53 100644 --- a/sdk/core/azure-mgmt-core/pyproject.toml +++ b/sdk/core/azure-mgmt-core/pyproject.toml @@ -4,3 +4,4 @@ verifytypes = true pyright = false type_check_samples = false breaking = false +black = true diff --git a/sdk/core/ci.yml b/sdk/core/ci.yml index fcc8caf21ecc..929efb3bf934 100644 --- a/sdk/core/ci.yml +++ b/sdk/core/ci.yml @@ -36,7 +36,6 @@ extends: parameters: ServiceDirectory: core BuildTargetingString: "*" - ValidateFormatting: true TestProxy: false TestTimeoutInMinutes: 120 Artifacts: diff --git a/sdk/core/corehttp/pyproject.toml b/sdk/core/corehttp/pyproject.toml index c3ad9f04d622..f5de7c105c5b 100644 --- a/sdk/core/corehttp/pyproject.toml +++ b/sdk/core/corehttp/pyproject.toml @@ -1,3 +1,4 @@ [tool.azure-sdk-build] pyright = false verify_keywords = false +black = true \ No newline at end of file diff --git a/sdk/evaluation/ci.yml b/sdk/evaluation/ci.yml index 628963561d6d..a9a2435d2041 100644 --- a/sdk/evaluation/ci.yml +++ b/sdk/evaluation/ci.yml @@ -26,7 +26,6 @@ extends: template: ../../eng/pipelines/templates/stages/archetype-sdk-client.yml parameters: ServiceDirectory: evaluation - ValidateFormatting: true TestProxy: true # This custom matrix config should be dropped once: # * Once azure-ai-ml supports 3.13 (currently crashes due to type annotation) diff --git a/sdk/identity/azure-identity-broker/pyproject.toml b/sdk/identity/azure-identity-broker/pyproject.toml index ea31fd0986d0..cf19e90d6267 100644 --- a/sdk/identity/azure-identity-broker/pyproject.toml +++ b/sdk/identity/azure-identity-broker/pyproject.toml @@ -1,3 +1,4 @@ [tool.azure-sdk-build] type_check_samples = false -pyright = false \ No newline at end of file +pyright = false +black = true \ No newline at end of file diff --git a/sdk/identity/azure-identity/pyproject.toml b/sdk/identity/azure-identity/pyproject.toml index 5047997ed4fd..25468ed89081 100644 --- a/sdk/identity/azure-identity/pyproject.toml +++ b/sdk/identity/azure-identity/pyproject.toml @@ -1,3 +1,4 @@ [tool.azure-sdk-build] pyright = false verifytypes = true +black = true \ No newline at end of file diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index 34f251d5f6a5..69cd4703775b 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -35,7 +35,6 @@ extends: Path: sdk/identity/platform-matrix.json Selection: sparse GenerateVMJobs: true - ValidateFormatting: true TestProxy: true Artifacts: - name: azure-identity diff --git a/sdk/ml/azure-ai-ml/pyproject.toml b/sdk/ml/azure-ai-ml/pyproject.toml index a46aa6bdd51f..1ee13a4913da 100644 --- a/sdk/ml/azure-ai-ml/pyproject.toml +++ b/sdk/ml/azure-ai-ml/pyproject.toml @@ -5,6 +5,7 @@ type_check_samples = false pylint = true mindependency = false latestdependency = false +black = true [tool.isort] profile = "black" diff --git a/sdk/ml/ci.yml b/sdk/ml/ci.yml index 5355efb7c85c..013df63cafb6 100644 --- a/sdk/ml/ci.yml +++ b/sdk/ml/ci.yml @@ -30,7 +30,6 @@ extends: template: /eng/pipelines/templates/stages/archetype-sdk-client.yml parameters: ServiceDirectory: ml - ValidateFormatting: true TestTimeoutInMinutes: 75 TestProxy: true MatrixFilters: diff --git a/sdk/personalizer/azure-ai-personalizer/pyproject.toml b/sdk/personalizer/azure-ai-personalizer/pyproject.toml index a3a58bd09030..c2179227be3e 100644 --- a/sdk/personalizer/azure-ai-personalizer/pyproject.toml +++ b/sdk/personalizer/azure-ai-personalizer/pyproject.toml @@ -4,3 +4,4 @@ pyright = false type_check_samples = false verifytypes = false ci_enabled = false +black = true \ No newline at end of file diff --git a/sdk/personalizer/ci.yml b/sdk/personalizer/ci.yml index 324316d85817..6026c98fbaf4 100644 --- a/sdk/personalizer/ci.yml +++ b/sdk/personalizer/ci.yml @@ -28,7 +28,6 @@ extends: parameters: ServiceDirectory: personalizer TestProxy: true - ValidateFormatting: true Artifacts: - name: azure-ai-personalizer safeName: azureaipersonalizer \ No newline at end of file diff --git a/sdk/pullrequest.yml b/sdk/pullrequest.yml index bf96c3d15417..9046b7ee59bd 100644 --- a/sdk/pullrequest.yml +++ b/sdk/pullrequest.yml @@ -24,6 +24,5 @@ extends: parameters: ServiceDirectory: ${{ parameters.Service }} BuildTargetingString: "*" - ValidateFormatting: true TestProxy: true TestTimeOutInMinutes: 180 \ No newline at end of file diff --git a/sdk/tables/azure-data-tables/pyproject.toml b/sdk/tables/azure-data-tables/pyproject.toml index e00361912969..2a482558eb95 100644 --- a/sdk/tables/azure-data-tables/pyproject.toml +++ b/sdk/tables/azure-data-tables/pyproject.toml @@ -1,2 +1,3 @@ [tool.azure-sdk-build] pyright = false +black = true \ No newline at end of file diff --git a/sdk/tables/ci.yml b/sdk/tables/ci.yml index 9d777407bddd..7430b8c77f67 100644 --- a/sdk/tables/ci.yml +++ b/sdk/tables/ci.yml @@ -30,7 +30,6 @@ extends: parameters: TestProxy: true ServiceDirectory: tables - ValidateFormatting: true Artifacts: - name: azure-data-tables safeName: azuredatatables diff --git a/sdk/template/ci.yml b/sdk/template/ci.yml index 558acd963c6e..784ec9a3ecd7 100644 --- a/sdk/template/ci.yml +++ b/sdk/template/ci.yml @@ -45,7 +45,6 @@ extends: parameters: oneESTemplateTag: ${{ parameters.oneESTemplateTag }} ServiceDirectory: template - ValidateFormatting: true TestProxy: true Artifacts: - name: azure-template diff --git a/tools/azure-sdk-tools/ci_tools/environment_exclusions.py b/tools/azure-sdk-tools/ci_tools/environment_exclusions.py index 728ddab87ee2..492804416cba 100644 --- a/tools/azure-sdk-tools/ci_tools/environment_exclusions.py +++ b/tools/azure-sdk-tools/ci_tools/environment_exclusions.py @@ -35,9 +35,10 @@ "azure-template", ] -MUST_RUN_ENVS = [ - "bandit" -] +MUST_RUN_ENVS = ["bandit"] + +# all of our checks default to ON, other than the below +CHECK_DEFAULTS = {"black": False} def is_check_enabled(package_path: str, check: str, default: Any = True) -> bool: """ @@ -81,7 +82,8 @@ def filter_tox_environment_string(namespace_argument: str, package_path: str) -> filtered_set = [] for tox_env in [env.strip().lower() for env in tox_envs]: - if is_check_enabled(package_path, tox_env, True) or tox_env in MUST_RUN_ENVS: + check_enabled = is_check_enabled(package_path, tox_env, CHECK_DEFAULTS.get(tox_env, True)) + if check_enabled or tox_env in MUST_RUN_ENVS: filtered_set.append(tox_env) return ",".join(filtered_set) diff --git a/tools/azure-sdk-tools/tests/integration/scenarios/ci_yml_present/service/ci.yml b/tools/azure-sdk-tools/tests/integration/scenarios/ci_yml_present/service/ci.yml index d358db016420..2d2a197682f6 100644 --- a/tools/azure-sdk-tools/tests/integration/scenarios/ci_yml_present/service/ci.yml +++ b/tools/azure-sdk-tools/tests/integration/scenarios/ci_yml_present/service/ci.yml @@ -36,7 +36,6 @@ extends: parameters: ServiceDirectory: core BuildTargetingString: "*" - ValidateFormatting: true TestProxy: false TestTimeoutInMinutes: 120 Artifacts: