Replace compile-time `validateFormatting` parameter (#38212)
* transition validate_formatting.py to run_black.py called from within the tox env * update filter_tox_env_string to handle check defaults that are nonTrue * default black to opt-in, not opt-out
This commit is contained in:
Scott Beddall
GitHub
Родитель
947ad7cdf8
Коммит
69f0c227c0
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 }}
|
||||
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
|
|||
|
|
@ -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'))
|
||||
|
|
@ -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}."
|
||||
)
|
||||
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
@ -1,2 +1,3 @@
|
|||
[tool.azure-sdk-build]
|
||||
pyright = false
|
||||
black = true
|
||||
|
|
@ -1,2 +1,3 @@
|
|||
[tool.azure-sdk-build]
|
||||
pyright = false
|
||||
black = true
|
||||
|
|
@ -31,7 +31,6 @@ extends:
|
|||
TestProxy: true
|
||||
ServiceDirectory: appconfiguration
|
||||
VerifyAutorest: false
|
||||
ValidateFormatting: true
|
||||
Artifacts:
|
||||
- name: azure-appconfiguration
|
||||
safeName: azureappconfiguration
|
||||
|
|
|
|||
|
|
@ -1,2 +1,3 @@
|
|||
[tool.azure-sdk-build]
|
||||
pyright = false
|
||||
black = true
|
||||
|
|
@ -1,2 +1,3 @@
|
|||
[tool.azure-sdk-build]
|
||||
breaking = false
|
||||
black = true
|
||||
|
|
@ -30,7 +30,6 @@ extends:
|
|||
parameters:
|
||||
TestProxy: true
|
||||
ServiceDirectory: containerregistry
|
||||
ValidateFormatting: true
|
||||
Artifacts:
|
||||
- name: azure-mgmt-containerregistry
|
||||
safeName: azuremgmtcontainerregistry
|
||||
|
|
|
|||
|
|
@ -2,3 +2,4 @@
|
|||
type_check_samples = false
|
||||
pyright = false
|
||||
mypy = true
|
||||
black = true
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
[tool.azure-sdk-build]
|
||||
type_check_samples = false
|
||||
pyright = false
|
||||
black = true
|
||||
|
|
@ -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.
|
||||
#
|
||||
|
|
|
|||
|
|
@ -4,3 +4,4 @@ verifytypes = true
|
|||
pyright = false
|
||||
type_check_samples = false
|
||||
breaking = false
|
||||
black = true
|
||||
|
|
|
|||
|
|
@ -36,7 +36,6 @@ extends:
|
|||
parameters:
|
||||
ServiceDirectory: core
|
||||
BuildTargetingString: "*"
|
||||
ValidateFormatting: true
|
||||
TestProxy: false
|
||||
TestTimeoutInMinutes: 120
|
||||
Artifacts:
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
[tool.azure-sdk-build]
|
||||
pyright = false
|
||||
verify_keywords = false
|
||||
black = true
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
[tool.azure-sdk-build]
|
||||
type_check_samples = false
|
||||
pyright = false
|
||||
pyright = false
|
||||
black = true
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
[tool.azure-sdk-build]
|
||||
pyright = false
|
||||
verifytypes = true
|
||||
black = true
|
||||
|
|
@ -35,7 +35,6 @@ extends:
|
|||
Path: sdk/identity/platform-matrix.json
|
||||
Selection: sparse
|
||||
GenerateVMJobs: true
|
||||
ValidateFormatting: true
|
||||
TestProxy: true
|
||||
Artifacts:
|
||||
- name: azure-identity
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ type_check_samples = false
|
|||
pylint = true
|
||||
mindependency = false
|
||||
latestdependency = false
|
||||
black = true
|
||||
|
||||
[tool.isort]
|
||||
profile = "black"
|
||||
|
|
|
|||
|
|
@ -30,7 +30,6 @@ extends:
|
|||
template: /eng/pipelines/templates/stages/archetype-sdk-client.yml
|
||||
parameters:
|
||||
ServiceDirectory: ml
|
||||
ValidateFormatting: true
|
||||
TestTimeoutInMinutes: 75
|
||||
TestProxy: true
|
||||
MatrixFilters:
|
||||
|
|
|
|||
|
|
@ -4,3 +4,4 @@ pyright = false
|
|||
type_check_samples = false
|
||||
verifytypes = false
|
||||
ci_enabled = false
|
||||
black = true
|
||||
|
|
@ -28,7 +28,6 @@ extends:
|
|||
parameters:
|
||||
ServiceDirectory: personalizer
|
||||
TestProxy: true
|
||||
ValidateFormatting: true
|
||||
Artifacts:
|
||||
- name: azure-ai-personalizer
|
||||
safeName: azureaipersonalizer
|
||||
|
|
@ -24,6 +24,5 @@ extends:
|
|||
parameters:
|
||||
ServiceDirectory: ${{ parameters.Service }}
|
||||
BuildTargetingString: "*"
|
||||
ValidateFormatting: true
|
||||
TestProxy: true
|
||||
TestTimeOutInMinutes: 180
|
||||
|
|
@ -1,2 +1,3 @@
|
|||
[tool.azure-sdk-build]
|
||||
pyright = false
|
||||
black = true
|
||||
|
|
@ -30,7 +30,6 @@ extends:
|
|||
parameters:
|
||||
TestProxy: true
|
||||
ServiceDirectory: tables
|
||||
ValidateFormatting: true
|
||||
Artifacts:
|
||||
- name: azure-data-tables
|
||||
safeName: azuredatatables
|
||||
|
|
|
|||
|
|
@ -45,7 +45,6 @@ extends:
|
|||
parameters:
|
||||
oneESTemplateTag: ${{ parameters.oneESTemplateTag }}
|
||||
ServiceDirectory: template
|
||||
ValidateFormatting: true
|
||||
TestProxy: true
|
||||
Artifacts:
|
||||
- name: azure-template
|
||||
|
|
|
|||
|
|
@ -35,9 +35,10 @@ IGNORE_PACKAGES = [
|
|||
"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)
|
||||
|
||||
|
|
|
|||
|
|
@ -36,7 +36,6 @@ extends:
|
|||
parameters:
|
||||
ServiceDirectory: core
|
||||
BuildTargetingString: "*"
|
||||
ValidateFormatting: true
|
||||
TestProxy: false
|
||||
TestTimeoutInMinutes: 120
|
||||
Artifacts:
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче