Skip to content
New issue

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

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

Already on GitHub? # to your account

Enable Pyproject Inclusion #28345

Merged
merged 46 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
7212ba5
work up the static file changes to support honoring pyproject.toml as…
scbedd Jan 6, 2023
5dc6943
add tomli to packaging deps. add pyproject.toml to spelling ignore list
scbedd Jan 6, 2023
42d2751
begin the process of removing OMITTED_PACKAGES in favor of only honor…
scbedd Jan 6, 2023
3f31011
now honoring the Inactive classifier over the omission list
scbedd Jan 6, 2023
f4248f9
apply black
scbedd Jan 7, 2023
bb7fd3a
we now honor the inactive classifier
scbedd Jan 7, 2023
b19f4f4
preparing to test
scbedd Jan 7, 2023
c0721c5
need to avoid the runtime error from the attempted parse
scbedd Jan 7, 2023
83b9c55
find a workaround for the package parsing of inactive packages
scbedd Jan 7, 2023
0becd05
tests now working
scbedd Jan 7, 2023
ace22ba
cut over to using the check override for regression. pretty much done…
scbedd Jan 7, 2023
743fbad
we now filter the environments BEFORE invoking tox. this will signifi…
scbedd Jan 9, 2023
dae9d53
ensure tests for package discovery are working
scbedd Jan 9, 2023
efe96a1
now we can enable/disable black on a per-package basis
scbedd Jan 9, 2023
90bb068
swap the positive and negative cases
scbedd Jan 9, 2023
545cdda
enable packages to enable/disable black on a per-package basis
scbedd Jan 9, 2023
81e2c61
enable black
scbedd Jan 10, 2023
ad4516c
comment update
scbedd Jan 10, 2023
853fb23
convert logging.info to logging.debug in functions.py
scbedd Jan 10, 2023
d8aa875
patch the validate_formatting script to output a bit of additional de…
scbedd Jan 10, 2023
4dc25b4
left a breakpoint in the code
scbedd Jan 11, 2023
1148f2a
Merge remote-tracking branch 'upstream/main' into enable-pyproject-in…
scbedd Jan 12, 2023
39a96aa
remove broken link from changelog
scbedd Jan 13, 2023
77e4036
Update tools/azure-sdk-tools/ci_tools/parsing/parse_functions.py
scbedd Jan 17, 2023
d1d3eab
Update tools/azure-sdk-tools/ci_tools/functions.py
scbedd Jan 17, 2023
1601fe5
Update tools/azure-sdk-tools/ci_tools/functions.py
scbedd Jan 17, 2023
a4b955e
Update tools/azure-sdk-tools/ci_tools/functions.py
scbedd Jan 17, 2023
4df4dc2
PR feedback from McCoy
scbedd Jan 17, 2023
46809fb
mgmt team accidentally broke sdk_packaging toml in azure-mgmt
scbedd Jan 17, 2023
c9ec7e8
solve cspell errors
scbedd Jan 17, 2023
c89c669
correct spelling errors in test_servicemetadata
scbedd Jan 17, 2023
19d8d22
Packaging update of azure-mgmt-scheduler
AutorestCI Jan 17, 2023
a65804e
Packaging update of azure-mgmt-regionmove
AutorestCI Jan 17, 2023
08dbc26
Revert "Packaging update of azure-mgmt-scheduler"
scbedd Jan 18, 2023
359e978
Revert "Packaging update of azure-mgmt-regionmove"
scbedd Jan 18, 2023
a5ba3f3
revert auto commits to azure-mgmt-scheduler and azure-mgmt-regionmove…
scbedd Jan 18, 2023
eef72c1
add detail to eng_sys_checks document
scbedd Jan 18, 2023
29c1e05
Merge remote-tracking branch 'upstream/main' into enable-pyproject-in…
scbedd Jan 18, 2023
7aab581
resolve final spelling issue
scbedd Jan 18, 2023
3f3ea5f
THIS may be the final cspell update
scbedd Jan 18, 2023
374489b
fix the failure in detect api changes
semick-dev Jan 19, 2023
dd33a06
identify issue with . as the package path
semick-dev Jan 19, 2023
3301048
patch get_package_properties
scbedd Jan 19, 2023
ed6d9e7
include inactive packages when assembling artifacts
scbedd Jan 19, 2023
8adb5e1
merge upstream to resolve merge conflict
scbedd Jan 19, 2023
7b028ec
resolve issue with inactive flag turning dev mode on
scbedd Jan 19, 2023
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
8 changes: 6 additions & 2 deletions .vscode/cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"sdk/**/_generated/**",
"**/*requirement*.txt",
"**/assets.json",

"**/pyproject.toml",
"sdk/anomalydetector/**",
"sdk/applicationinsights/azure-applicationinsights/**",
"sdk/batch/azure-batch/**",
Expand Down Expand Up @@ -95,9 +95,13 @@
".gitignore",
"tools/azure-sdk-tools/devtools_testutils/fake_credentials.py",
"tools/azure-sdk-tools/packaging_tools/**",
"tools/azure-sdk-tools/setup.py"
"tools/azure-sdk-tools/setup.py",
"tools/azure-sdk-tools/tests/test_servicemetadata.py"
],
"words": [
"pyprojecttoml",
"tomli",
"sdkrel",
"qnamaker",
"mindependency",
"automl",
Expand Down
51 changes: 42 additions & 9 deletions doc/eng_sys_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
- [Targeting a specific package at build queue time](#targeting-a-specific-package-at-build-queue-time)
- [Skipping a tox test environment at build queue time](#skipping-a-tox-test-environment-at-build-queue-time)
- [Skipping entire sections of builds](#skipping-entire-sections-of-builds)
- [The pyproject.toml](#the-pyprojecttoml)
- [Environment variables important to CI](#environment-variables-important-to-ci)
- [Atomic Overrides](#atomic-overrides)
- [Analyze Checks](#analyze-checks)
- [MyPy](#mypy)
- [Pylint](#pylint)
Expand Down Expand Up @@ -102,6 +104,28 @@ This is the most useful skip, but the following skip variables are also supporte
- `Skip.VerifyDependencies`
- Omit checking that a package's dependencies are on PyPI before releasing.

## The pyproject.toml

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.

We default to **enabling** most of our checks like `pylint`, `mypy`, etc. Due to that, most `pyproject.toml` settings will likely be **disabling** checks.

Here's an example:

```toml
# from sdk/core/azure-servicemanagement-legacy/pyproject.toml, which is a legacy package
# as a result, all of these checks are disabled
[tool.azure-sdk-build]
mypy = false
type_check_samples = false
verifytypes = false
pyright = false
pylint = false
black = 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.

## Environment variables important to CI

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.
Expand All @@ -114,6 +138,22 @@ There are a few differences from a standard local invocation of `tox <env>`. Pri

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

### Atomic Overrides

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.

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.

- `ENABLE_AZURE_COMMON=true`
- `ENABLE_AZURE_SERVICEMANAGEMENT_LEGACY=true`

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`.

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:

- `AZURE_SERVICEBUS_PYRIGHT=true` <-- enable a check that normally is disabled in `pyproject.toml`
- `AZURE_CORE_PYLINT=false` <-- disable a check that normally runs

## Analyze Checks

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.
Expand All @@ -122,15 +162,8 @@ Analyze job in both nightly CI and pull request validation pipeline runs a set o

[`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:

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:
```python
MYPY_HARD_FAILURE_OPTED = [
...,
"azure-my-package",
]
```
2. Go to root of the package
3. Execute following command: `tox -e mypy -c ../../../eng/tox/tox.ini`
1. Go to root of the package
2. Execute following command: `tox -e mypy -c ../../../eng/tox/tox.ini`

### Pylint

Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/templates/steps/build-artifacts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ steps:
condition: and(succeeded(),eq(variables['SetDevVersion'],'true'))

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

Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/templates/steps/run_black.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ steps:
condition: succeededOrFailed()

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

Expand Down
1 change: 1 addition & 0 deletions eng/scripts/get_package_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
if "setup.py" in files:
try:
parsed = ParsedSetup.from_path(root)

print(
"{0} {1} {2} {3}".format(
parsed.name, parsed.version, parsed.is_new_sdk, os.path.dirname(parsed.setup_filename)
Expand Down
7 changes: 3 additions & 4 deletions eng/tox/run_mypy.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@

from ci_tools.environment_exclusions import (
is_ignored_package,
MYPY_OPT_OUT,
TYPE_CHECK_SAMPLES_OPT_OUT,
is_check_enabled
)

logging.getLogger().setLevel(logging.INFO)
Expand All @@ -36,7 +35,7 @@

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

if package_name in TYPE_CHECK_SAMPLES_OPT_OUT:
if not is_check_enabled(args.target_package, "type_check_samples", True):
logging.info(
f"Package {package_name} opts-out of mypy check on samples."
)
Expand Down
4 changes: 2 additions & 2 deletions eng/tox/run_pylint.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import logging
import sys

from ci_tools.environment_exclusions import PYLINT_OPT_OUT
from ci_tools.environment_exclusions import is_check_enabled
from ci_tools.parsing import ParsedSetup

logging.getLogger().setLevel(logging.INFO)
Expand Down Expand Up @@ -43,7 +43,7 @@

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

if pkg_details.name not in PYLINT_OPT_OUT:
if is_check_enabled(args.target_package, "pylint"):
try:
check_call(
[
Expand Down
7 changes: 3 additions & 4 deletions eng/tox/run_pyright.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@

from ci_tools.environment_exclusions import (
is_ignored_package,
PYRIGHT_OPT_OUT,
TYPE_CHECK_SAMPLES_OPT_OUT,
is_check_enabled,
)

logging.getLogger().setLevel(logging.INFO)
Expand All @@ -36,7 +35,7 @@

args = parser.parse_args()
package_name = os.path.basename(os.path.abspath(args.target_package))
if package_name in PYRIGHT_OPT_OUT or is_ignored_package(package_name):
if not is_check_enabled(args.target_package, "pyright") or is_ignored_package(package_name):
logging.info(
f"Package {package_name} opts-out of pyright check. See https://aka.ms/python/typing-guide for information."
)
Expand All @@ -46,7 +45,7 @@
os.path.join(args.target_package, "azure"),
os.path.join(args.target_package, "samples"),
]
if package_name in TYPE_CHECK_SAMPLES_OPT_OUT:
if not is_check_enabled(args.target_package, "type_check_samples"):
logging.info(
f"Package {package_name} opts-out of pyright check on samples."
)
Expand Down
4 changes: 2 additions & 2 deletions eng/tox/run_verifytypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import logging
import sys

from ci_tools.environment_exclusions import is_ignored_package, VERIFYTYPES_OPT_OUT
from ci_tools.environment_exclusions import is_ignored_package, is_check_enabled

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

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

if package_name in VERIFYTYPES_OPT_OUT or is_ignored_package(package_name):
if not is_check_enabled(args.target_package, "type_check_samples") or is_ignored_package(package_name):
logging.info(
f"{package_name} opts-out of verifytypes check. See https://aka.ms/python/typing-guide for information."
)
Expand Down
1 change: 1 addition & 0 deletions eng/tox/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pkgs =
wheel==0.37.0
packaging==20.4
urllib3==1.26.12
tomli==2.0.1


[testenv]
Expand Down
15 changes: 1 addition & 14 deletions scripts/devops_tasks/common_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from packaging.version import Version
from packaging.version import parse

from ci_tools.functions import MANAGEMENT_PACKAGE_IDENTIFIERS, lambda_filter_azure_pkg
from ci_tools.functions import MANAGEMENT_PACKAGE_IDENTIFIERS, lambda_filter_azure_pkg, str_to_bool
from ci_tools.parsing import parse_require, ParsedSetup

DEV_REQ_FILE = "dev_requirements.txt"
Expand All @@ -41,8 +41,6 @@
logging.getLogger().setLevel(logging.INFO)




def log_file(file_location, is_error=False):
with open(file_location, "r") as file:
for line in file:
Expand Down Expand Up @@ -81,17 +79,6 @@ def clean_coverage(coverage_dir):
raise


def str_to_bool(input_string):
if isinstance(input_string, bool):
return input_string
elif input_string.lower() in ("true", "t", "1"):
return True
elif input_string.lower() in ("false", "f", "0"):
return False
else:
return False


def run_check_call(
command_array,
working_directory,
Expand Down
5 changes: 2 additions & 3 deletions scripts/devops_tasks/test_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
find_whl,
find_tools_packages,
get_installed_packages,
extend_dev_requirements,
str_to_bool,
extend_dev_requirements
)

from git_helper import (
Expand All @@ -34,7 +33,7 @@
clone_repo,
)

from ci_tools.functions import discover_targeted_packages
from ci_tools.functions import discover_targeted_packages, str_to_bool
from ci_tools.parsing import ParsedSetup

AZURE_GLOB_STRING = "azure*"
Expand Down
4 changes: 2 additions & 2 deletions scripts/devops_tasks/tox_harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,12 +323,12 @@ def prep_and_run_tox(targeted_packages: List[str], parsed_args: Namespace, optio
inject_custom_reqs(destination_dev_req, parsed_args.injected_packages, package_dir)

if parsed_args.tox_env:
filtered_tox_environment_set = filter_tox_environment_string(parsed_args.tox_env, package_name)
filtered_tox_environment_set = filter_tox_environment_string(parsed_args.tox_env, package_dir)

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

Expand Down
43 changes: 30 additions & 13 deletions scripts/devops_tasks/validate_formatting.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,40 @@
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(service_dir):
results = []
logging.info("Running black for {}".format(service_dir))

out = subprocess.Popen([sys.executable, "-m", "black", "-l", "120", "sdk/{}".format(service_dir)],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
cwd = root_dir
)
discovered_packages = discover_targeted_packages("azure*", os.path.join(root_dir, "sdk", service_dir))

for package in discovered_packages:
package_name = os.path.basename(package)

if is_check_enabled(package, "black", True):
out = subprocess.Popen([sys.executable, "-m", "black", "-l", "120", "sdk/{}/{}".format(service_dir, package_name)],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
cwd = root_dir
)

stdout,stderr = out.communicate()
stdout,stderr = out.communicate()

if stderr:
raise RuntimeError("black ran into some trouble during its invocation: " + stderr)
if stderr:
results.append((package_name, stderr))

if stdout:
if "reformatted" in stdout.decode('utf-8'):
return False
if stdout:
if "reformatted" in stdout.decode('utf-8'):
results.append((package_name, False))
else:
print(f"black succeeded against {package_name}")

return True
return results


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

args = parser.parse_args()
if args.validate != "False":
if not run_black(args.service_directory):
results = run_black(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. Please re-generate with the latest autorest.")

else:
Expand Down
8 changes: 8 additions & 0 deletions sdk/core/azure-common/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[tool.azure-sdk-build]
type_check_samples = false
verifytypes = false
pyright = false
mypy = false
pylint = false
regression = false
black = false
5 changes: 5 additions & 0 deletions sdk/core/azure-core-experimental/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[tool.azure-sdk-build]
type_check_samples = false
verifytypes = false
pyright = false
mypy = false
5 changes: 5 additions & 0 deletions sdk/core/azure-core-tracing-opencensus/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[tool.azure-sdk-build]
type_check_samples = false
verifytypes = false
pyright = false
mypy = false
5 changes: 5 additions & 0 deletions sdk/core/azure-core-tracing-opentelemetry/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[tool.azure-sdk-build]
type_check_samples = false
verifytypes = false
pyright = false
mypy = false
Loading