Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace compile-time validateFormatting parameter #38212

Merged
merged 7 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
18 changes: 10 additions & 8 deletions doc/eng_sys_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand Down
4 changes: 0 additions & 4 deletions eng/pipelines/templates/jobs/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ parameters:
- name: VerifyAutorest
type: boolean
default: false
- name: ValidateFormatting
type: boolean
default: false
- name: UnsupportedToxEnvironments
type: string
default: ''
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions eng/pipelines/templates/stages/archetype-sdk-client.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ parameters:
- name: VerifyAutorest
type: boolean
default: false
- name: ValidateFormatting
type: boolean
default: false
- name: TestProxy
type: boolean
default: true
Expand Down Expand Up @@ -107,7 +104,6 @@ extends:
MatrixFilters: ${{ parameters.MatrixFilters }}
MatrixReplace: ${{ parameters.MatrixReplace }}
VerifyAutorest: ${{ parameters.VerifyAutorest }}
ValidateFormatting: ${{ parameters.ValidateFormatting }}
TestProxy: ${{ parameters.TestProxy }}
GenerateApiReviewForManualOnly: ${{ parameters.GenerateApiReviewForManualOnly }}

Expand Down
9 changes: 3 additions & 6 deletions eng/pipelines/templates/steps/analyze.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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'
Expand Down
12 changes: 7 additions & 5 deletions eng/pipelines/templates/steps/run_black.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
parameters:
ServiceDirectory: ''
ValidateFormatting: false
EnvVars: {}
AdditionalTestArgs: ''

Expand All @@ -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'))
67 changes: 67 additions & 0 deletions eng/tox/run_black.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#!/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.

from subprocess import check_call, CalledProcessError
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:
check_call(
[
sys.executable,
"-m",
"black",
f"--config={configFileLocation}",
args.target_package,
]
)
except CalledProcessError as e:
logging.error(
f"{pkg_details.name} exited with black error {e.returncode}." +
"This package is explicitly enabled for black within the package's pyproject.toml."
)

exit(1)
4 changes: 2 additions & 2 deletions eng/tox/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
107 changes: 0 additions & 107 deletions scripts/devops_tasks/validate_formatting.py

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[tool.azure-sdk-build]
pyright = false
black = true
1 change: 1 addition & 0 deletions sdk/appconfiguration/azure-appconfiguration/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[tool.azure-sdk-build]
pyright = false
black = true
1 change: 0 additions & 1 deletion sdk/appconfiguration/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ extends:
TestProxy: true
ServiceDirectory: appconfiguration
VerifyAutorest: false
ValidateFormatting: true
Artifacts:
- name: azure-appconfiguration
safeName: azureappconfiguration
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[tool.azure-sdk-build]
pyright = false
black = true
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[tool.azure-sdk-build]
breaking = false
black = true
1 change: 0 additions & 1 deletion sdk/containerregistry/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ extends:
parameters:
TestProxy: true
ServiceDirectory: containerregistry
ValidateFormatting: true
Artifacts:
- name: azure-mgmt-containerregistry
safeName: azuremgmtcontainerregistry
Expand Down
1 change: 1 addition & 0 deletions sdk/core/azure-core-experimental/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
type_check_samples = false
pyright = false
mypy = true
black = true
1 change: 1 addition & 0 deletions sdk/core/azure-core-tracing-opentelemetry/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[tool.azure-sdk-build]
type_check_samples = false
pyright = false
black = true
1 change: 1 addition & 0 deletions sdk/core/azure-core/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
1 change: 1 addition & 0 deletions sdk/core/azure-mgmt-core/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ verifytypes = true
pyright = false
type_check_samples = false
breaking = false
black = true
1 change: 0 additions & 1 deletion sdk/core/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ extends:
parameters:
ServiceDirectory: core
BuildTargetingString: "*"
ValidateFormatting: true
TestProxy: false
TestTimeoutInMinutes: 120
Artifacts:
Expand Down
1 change: 1 addition & 0 deletions sdk/core/corehttp/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[tool.azure-sdk-build]
pyright = false
verify_keywords = false
black = true
1 change: 0 additions & 1 deletion sdk/evaluation/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading