Skip to content

Commit d0f6d33

Browse files
scbeddmccoyp
andauthored
Enable Pyproject Inclusion (#28345)
* inactive packages are still assembled, but will not be automatically tested unless included using ENABLE_PACKAGE_NAME=true * checks can now be individually disabled through the pyproject.toml * added package classifiers to ParsedSetup properties * add new function is_check_enabled which combines environment_exclusions, pyproject.toml, and environment overrides into one slick package * all run_check scripts now honor is_check_enabled to enable/disable atomically * update eng_sys_checks for new functionality * add pyproject.toml for all core/ packages * enable black on only track 2 core packages * remove package build exclusion list in favor of honoring classifier * update a couple packages with Inactive classifier so they are no longer built. azure-mgmt-scheduler, azure-mgmt-documentdb, azure-mgmt-regionmove Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>
1 parent d0d7442 commit d0f6d33

File tree

41 files changed

+492
-165
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+492
-165
lines changed

.vscode/cspell.json

+6-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"sdk/**/_generated/**",
2828
"**/*requirement*.txt",
2929
"**/assets.json",
30-
30+
"**/pyproject.toml",
3131
"sdk/anomalydetector/**",
3232
"sdk/applicationinsights/azure-applicationinsights/**",
3333
"sdk/batch/azure-batch/**",
@@ -95,9 +95,13 @@
9595
".gitignore",
9696
"tools/azure-sdk-tools/devtools_testutils/fake_credentials.py",
9797
"tools/azure-sdk-tools/packaging_tools/**",
98-
"tools/azure-sdk-tools/setup.py"
98+
"tools/azure-sdk-tools/setup.py",
99+
"tools/azure-sdk-tools/tests/test_servicemetadata.py"
99100
],
100101
"words": [
102+
"pyprojecttoml",
103+
"tomli",
104+
"sdkrel",
101105
"qnamaker",
102106
"mindependency",
103107
"automl",

doc/eng_sys_checks.md

+42-9
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
- [Targeting a specific package at build queue time](#targeting-a-specific-package-at-build-queue-time)
55
- [Skipping a tox test environment at build queue time](#skipping-a-tox-test-environment-at-build-queue-time)
66
- [Skipping entire sections of builds](#skipping-entire-sections-of-builds)
7+
- [The pyproject.toml](#the-pyprojecttoml)
78
- [Environment variables important to CI](#environment-variables-important-to-ci)
9+
- [Atomic Overrides](#atomic-overrides)
810
- [Analyze Checks](#analyze-checks)
911
- [MyPy](#mypy)
1012
- [Pylint](#pylint)
@@ -102,6 +104,28 @@ This is the most useful skip, but the following skip variables are also supporte
102104
- `Skip.VerifyDependencies`
103105
- Omit checking that a package's dependencies are on PyPI before releasing.
104106

107+
## The pyproject.toml
108+
109+
Starting with [this pr](https://github.com/Azure/azure-sdk-for-python/pull/28345), which checks apply to which packages are now **established** in a `pyproject.toml`, right next to each package's `setup.py`. This not only allows devs to fine-tune which checks that are applied at a package-level, but also seriously reduces confusion as to which checks apply when.
110+
111+
We default to **enabling** most of our checks like `pylint`, `mypy`, etc. Due to that, most `pyproject.toml` settings will likely be **disabling** checks.
112+
113+
Here's an example:
114+
115+
```toml
116+
# from sdk/core/azure-servicemanagement-legacy/pyproject.toml, which is a legacy package
117+
# as a result, all of these checks are disabled
118+
[tool.azure-sdk-build]
119+
mypy = false
120+
type_check_samples = false
121+
verifytypes = false
122+
pyright = false
123+
pylint = false
124+
black = false
125+
```
126+
127+
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.
128+
105129
## Environment variables important to CI
106130

107131
There are a few differences from a standard local invocation of `tox <env>`. Primarily, these differences adjust the checks to be friendly to parallel invocation. These adjustments are necessary to prevent random CI crashes.
@@ -114,6 +138,22 @@ There are a few differences from a standard local invocation of `tox <env>`. Pri
114138

115139
The various tooling abstracted by the environments within `eng/tox/tox.ini` take the above variables into account automatically.
116140

141+
### Atomic Overrides
142+
143+
Packages with classifier `Development Status :: 7 - Inactive`, are **not** built by default and as such normal `checks` like `mypy` and `pylint` are also not run against them. Older "core" packages like `azure-common` and `azure-servicemanagement-legacy` are present, but excluded from the build due to this restriction.
144+
145+
To temporarily **override** this restriction, a dev need only set the queue time variable: `ENABLE_PACKAGE_NAME`. The `-` in package names should be replaced by an `_`, as that is how the environment variable will be set on the actual CI machine anyway.
146+
147+
- `ENABLE_AZURE_COMMON=true`
148+
- `ENABLE_AZURE_SERVICEMANAGEMENT_LEGACY=true`
149+
150+
This same methodology also applies to _individual checks_ that run during various phases of CI. Developers can use a queue time variable of format `PACKAGE_NAME_CHECK=true/false`.
151+
152+
The name that you should use is visible based on what the `tox environment` that the check refers to! Here are a few examples of enabling/disabling checks:
153+
154+
- `AZURE_SERVICEBUS_PYRIGHT=true` <-- enable a check that normally is disabled in `pyproject.toml`
155+
- `AZURE_CORE_PYLINT=false` <-- disable a check that normally runs
156+
117157
## Analyze Checks
118158

119159
Analyze job in both nightly CI and pull request validation pipeline runs a set of static analysis using external and internal tools. Following are the list of these static analysis.
@@ -122,15 +162,8 @@ Analyze job in both nightly CI and pull request validation pipeline runs a set o
122162

123163
[`MyPy`](https://pypi.org/project/mypy/) is a static analysis tool that runs type checking of python package. MyPy is an opt-in check for packages. Following are the steps to run `MyPy` locally for a specific package:
124164

125-
1. Add the package name to the end of the [`mypy_hard_failure_packages.py`](https://github.com/Azure/azure-sdk-for-python/blob/main/eng/tox/mypy_hard_failure_packages.py) file:
126-
```python
127-
MYPY_HARD_FAILURE_OPTED = [
128-
...,
129-
"azure-my-package",
130-
]
131-
```
132-
2. Go to root of the package
133-
3. Execute following command: `tox -e mypy -c ../../../eng/tox/tox.ini`
165+
1. Go to root of the package
166+
2. Execute following command: `tox -e mypy -c ../../../eng/tox/tox.ini`
134167

135168
### Pylint
136169

eng/pipelines/templates/steps/build-artifacts.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ steps:
6363
condition: and(succeeded(),eq(variables['SetDevVersion'],'true'))
6464

6565
- pwsh: |
66-
sdk_build -d "$(Build.ArtifactStagingDirectory)" "$(TargetingString)" --service=${{parameters.ServiceDirectory}}
66+
sdk_build -d "$(Build.ArtifactStagingDirectory)" "$(TargetingString)" --service=${{parameters.ServiceDirectory}} --inactive
6767
displayName: 'Generate Packages'
6868
condition: succeededOrFailed()
6969

eng/pipelines/templates/steps/run_black.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ steps:
1111
condition: succeededOrFailed()
1212

1313
- script: |
14-
pip install black==21.6b0
14+
pip install black==21.6b0 tools/azure-sdk-tools["build"]
1515
displayName: 'Prep Environment'
1616
condition: succeededOrFailed()
1717

eng/scripts/get_package_properties.py

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
if "setup.py" in files:
1818
try:
1919
parsed = ParsedSetup.from_path(root)
20+
2021
print(
2122
"{0} {1} {2} {3}".format(
2223
parsed.name, parsed.version, parsed.is_new_sdk, os.path.dirname(parsed.setup_filename)

eng/tox/run_mypy.py

+3-4
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515

1616
from ci_tools.environment_exclusions import (
1717
is_ignored_package,
18-
MYPY_OPT_OUT,
19-
TYPE_CHECK_SAMPLES_OPT_OUT,
18+
is_check_enabled
2019
)
2120

2221
logging.getLogger().setLevel(logging.INFO)
@@ -36,7 +35,7 @@
3635

3736
args = parser.parse_args()
3837
package_name = os.path.basename(os.path.abspath(args.target_package))
39-
if package_name in MYPY_OPT_OUT or is_ignored_package(package_name):
38+
if not is_check_enabled(args.target_package, "mypy", True) or is_ignored_package(package_name):
4039
logging.info(
4140
f"Package {package_name} opts-out of mypy check. See https://aka.ms/python/typing-guide for information."
4241
)
@@ -62,7 +61,7 @@
6261
except CalledProcessError as src_err:
6362
src_code_error = src_err
6463

65-
if package_name in TYPE_CHECK_SAMPLES_OPT_OUT:
64+
if not is_check_enabled(args.target_package, "type_check_samples", True):
6665
logging.info(
6766
f"Package {package_name} opts-out of mypy check on samples."
6867
)

eng/tox/run_pylint.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import logging
1515
import sys
1616

17-
from ci_tools.environment_exclusions import PYLINT_OPT_OUT
17+
from ci_tools.environment_exclusions import is_check_enabled
1818
from ci_tools.parsing import ParsedSetup
1919

2020
logging.getLogger().setLevel(logging.INFO)
@@ -43,7 +43,7 @@
4343

4444
top_level_module = pkg_details.namespace.split('.')[0]
4545

46-
if pkg_details.name not in PYLINT_OPT_OUT:
46+
if is_check_enabled(args.target_package, "pylint"):
4747
try:
4848
check_call(
4949
[

eng/tox/run_pyright.py

+3-4
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515

1616
from ci_tools.environment_exclusions import (
1717
is_ignored_package,
18-
PYRIGHT_OPT_OUT,
19-
TYPE_CHECK_SAMPLES_OPT_OUT,
18+
is_check_enabled,
2019
)
2120

2221
logging.getLogger().setLevel(logging.INFO)
@@ -36,7 +35,7 @@
3635

3736
args = parser.parse_args()
3837
package_name = os.path.basename(os.path.abspath(args.target_package))
39-
if package_name in PYRIGHT_OPT_OUT or is_ignored_package(package_name):
38+
if not is_check_enabled(args.target_package, "pyright") or is_ignored_package(package_name):
4039
logging.info(
4140
f"Package {package_name} opts-out of pyright check. See https://aka.ms/python/typing-guide for information."
4241
)
@@ -46,7 +45,7 @@
4645
os.path.join(args.target_package, "azure"),
4746
os.path.join(args.target_package, "samples"),
4847
]
49-
if package_name in TYPE_CHECK_SAMPLES_OPT_OUT:
48+
if not is_check_enabled(args.target_package, "type_check_samples"):
5049
logging.info(
5150
f"Package {package_name} opts-out of pyright check on samples."
5251
)

eng/tox/run_verifytypes.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import logging
1717
import sys
1818

19-
from ci_tools.environment_exclusions import is_ignored_package, VERIFYTYPES_OPT_OUT
19+
from ci_tools.environment_exclusions import is_ignored_package, is_check_enabled
2020

2121
logging.getLogger().setLevel(logging.INFO)
2222

@@ -98,7 +98,7 @@ def get_type_complete_score(commands, check_pytyped=False):
9898
module = package_name.replace("-", ".")
9999
setup_path = os.path.abspath(args.target_package)
100100

101-
if package_name in VERIFYTYPES_OPT_OUT or is_ignored_package(package_name):
101+
if not is_check_enabled(args.target_package, "type_check_samples") or is_ignored_package(package_name):
102102
logging.info(
103103
f"{package_name} opts-out of verifytypes check. See https://aka.ms/python/typing-guide for information."
104104
)

eng/tox/tox.ini

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ pkgs =
3939
wheel==0.37.0
4040
packaging==20.4
4141
urllib3==1.26.12
42+
tomli==2.0.1
4243

4344

4445
[testenv]

scripts/devops_tasks/common_tasks.py

+1-14
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
from packaging.version import Version
3232
from packaging.version import parse
3333

34-
from ci_tools.functions import MANAGEMENT_PACKAGE_IDENTIFIERS, lambda_filter_azure_pkg
34+
from ci_tools.functions import MANAGEMENT_PACKAGE_IDENTIFIERS, lambda_filter_azure_pkg, str_to_bool
3535
from ci_tools.parsing import parse_require, ParsedSetup
3636

3737
DEV_REQ_FILE = "dev_requirements.txt"
@@ -41,8 +41,6 @@
4141
logging.getLogger().setLevel(logging.INFO)
4242

4343

44-
45-
4644
def log_file(file_location, is_error=False):
4745
with open(file_location, "r") as file:
4846
for line in file:
@@ -81,17 +79,6 @@ def clean_coverage(coverage_dir):
8179
raise
8280

8381

84-
def str_to_bool(input_string):
85-
if isinstance(input_string, bool):
86-
return input_string
87-
elif input_string.lower() in ("true", "t", "1"):
88-
return True
89-
elif input_string.lower() in ("false", "f", "0"):
90-
return False
91-
else:
92-
return False
93-
94-
9582
def run_check_call(
9683
command_array,
9784
working_directory,

scripts/devops_tasks/test_regression.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@
2323
find_whl,
2424
find_tools_packages,
2525
get_installed_packages,
26-
extend_dev_requirements,
27-
str_to_bool,
26+
extend_dev_requirements
2827
)
2928

3029
from git_helper import (
@@ -34,7 +33,7 @@
3433
clone_repo,
3534
)
3635

37-
from ci_tools.functions import discover_targeted_packages
36+
from ci_tools.functions import discover_targeted_packages, str_to_bool
3837
from ci_tools.parsing import ParsedSetup
3938

4039
AZURE_GLOB_STRING = "azure*"

scripts/devops_tasks/tox_harness.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,12 @@ def prep_and_run_tox(targeted_packages: List[str], parsed_args: Namespace, optio
323323
inject_custom_reqs(destination_dev_req, parsed_args.injected_packages, package_dir)
324324

325325
if parsed_args.tox_env:
326-
filtered_tox_environment_set = filter_tox_environment_string(parsed_args.tox_env, package_name)
326+
filtered_tox_environment_set = filter_tox_environment_string(parsed_args.tox_env, package_dir)
327327

328328
if not filtered_tox_environment_set:
329329
logging.info(
330330
f"All requested tox environments for package {package_name} have been excluded by the environment exclusion list."
331-
+ " Check file /tools/azure-sdk-tools/ci_tools/environment_exclusions.py"
331+
+ " Check file /tools/azure-sdk-tools/ci_tools/environment_exclusions.py and the pyproject.toml."
332332
)
333333
continue
334334

scripts/devops_tasks/validate_formatting.py

+30-13
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,40 @@
1212
import subprocess
1313

1414
logging.getLogger().setLevel(logging.INFO)
15+
from ci_tools.functions import discover_targeted_packages
16+
from ci_tools.environment_exclusions import is_check_enabled
1517

1618
root_dir = os.path.abspath(os.path.join(os.path.abspath(__file__), "..", "..", ".."))
1719
sdk_dir = os.path.join(root_dir, "sdk")
1820

1921
def run_black(service_dir):
22+
results = []
2023
logging.info("Running black for {}".format(service_dir))
2124

22-
out = subprocess.Popen([sys.executable, "-m", "black", "-l", "120", "sdk/{}".format(service_dir)],
23-
stdout=subprocess.PIPE,
24-
stderr=subprocess.STDOUT,
25-
cwd = root_dir
26-
)
25+
discovered_packages = discover_targeted_packages("azure*", os.path.join(root_dir, "sdk", service_dir))
26+
27+
for package in discovered_packages:
28+
package_name = os.path.basename(package)
29+
30+
if is_check_enabled(package, "black", True):
31+
out = subprocess.Popen([sys.executable, "-m", "black", "-l", "120", "sdk/{}/{}".format(service_dir, package_name)],
32+
stdout=subprocess.PIPE,
33+
stderr=subprocess.STDOUT,
34+
cwd = root_dir
35+
)
2736

28-
stdout,stderr = out.communicate()
37+
stdout,stderr = out.communicate()
2938

30-
if stderr:
31-
raise RuntimeError("black ran into some trouble during its invocation: " + stderr)
39+
if stderr:
40+
results.append((package_name, stderr))
3241

33-
if stdout:
34-
if "reformatted" in stdout.decode('utf-8'):
35-
return False
42+
if stdout:
43+
if "reformatted" in stdout.decode('utf-8'):
44+
results.append((package_name, False))
45+
else:
46+
print(f"black succeeded against {package_name}")
3647

37-
return True
48+
return results
3849

3950

4051
if __name__ == "__main__":
@@ -51,7 +62,13 @@ def run_black(service_dir):
5162

5263
args = parser.parse_args()
5364
if args.validate != "False":
54-
if not run_black(args.service_directory):
65+
results = run_black(args.service_directory)
66+
67+
if len(results) > 0:
68+
for result in results:
69+
error = "Code needs reformat." if result[1] == False else error
70+
logging.error(f"Black run for {result[0]} ran into an issue: {error}")
71+
5572
raise ValueError("Found difference between formatted code and current commit. Please re-generate with the latest autorest.")
5673

5774
else:

sdk/core/azure-common/pyproject.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[tool.azure-sdk-build]
2+
type_check_samples = false
3+
verifytypes = false
4+
pyright = false
5+
mypy = false
6+
pylint = false
7+
regression = false
8+
black = false
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[tool.azure-sdk-build]
2+
type_check_samples = false
3+
verifytypes = false
4+
pyright = false
5+
mypy = false
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[tool.azure-sdk-build]
2+
type_check_samples = false
3+
verifytypes = false
4+
pyright = false
5+
mypy = false
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[tool.azure-sdk-build]
2+
type_check_samples = false
3+
verifytypes = false
4+
pyright = false
5+
mypy = false

0 commit comments

Comments
 (0)