diff --git a/.github/workflows/scripts-test.yaml b/.github/workflows/scripts-test.yaml index e11424924..9c7f1ace5 100644 --- a/.github/workflows/scripts-test.yaml +++ b/.github/workflows/scripts-test.yaml @@ -48,6 +48,9 @@ jobs: - name: Check code health run: python $scripts_validation_dir/code_health.py -i . + + - name: Check code documentation + run: python $scripts_validation_dir/doc_style.py -i . test: name: Test scripts diff --git a/benchmark/flake_rules.json b/benchmark/validation_rules.json similarity index 74% rename from benchmark/flake_rules.json rename to benchmark/validation_rules.json index 3a98aec8a..97be3cf88 100644 --- a/benchmark/flake_rules.json +++ b/benchmark/validation_rules.json @@ -9,9 +9,18 @@ "src/components/pytorch_image_classifier/torch_helper/model/__init__.py:F401", "src/components/tensorflow_image_segmentation/train.py:E402" ], + "max-line-length": 200, "justifications": [ "__init__.py - namespace level files excluded from validation", "E402 - allow module level imports for specific files" ] + }, + "doc": { + "exclude": [ + "." + ], + "justifications": [ + ". - not enabled yet" + ] } -} \ No newline at end of file +} diff --git a/responsibleai/validation_rules.json b/responsibleai/validation_rules.json new file mode 100644 index 000000000..174c66ceb --- /dev/null +++ b/responsibleai/validation_rules.json @@ -0,0 +1,13 @@ +{ + "pep8": { + "max-line-length": 200 + }, + "doc": { + "exclude": [ + "." + ], + "justifications": [ + ". - not enabled yet" + ] + } +} diff --git a/scripts/azureml-assets/azureml/assets/config.py b/scripts/azureml-assets/azureml/assets/config.py index b16e6f408..ca781b343 100644 --- a/scripts/azureml-assets/azureml/assets/config.py +++ b/scripts/azureml-assets/azureml/assets/config.py @@ -173,7 +173,7 @@ class EnvironmentConfig(Config): Example: image: - name: azureml/curated/tensorflow-2.7-ubuntu20.04-py38-cuda11-gpu # Can include registry hostname and template tags + name: azureml/curated/tensorflow-2.7-ubuntu20.04-py38-cuda11-gpu # Can include registry hostname & template tags os: linux context: # If not specified, image won't be built dir: context @@ -414,7 +414,8 @@ class AssetConfig(Config): if not Config._is_set(name): name = self.spec_as_object().name if Config._contains_template(name): - raise ValidationException(f"Tried to read asset name from spec, but it includes a template tag: {name}") + raise ValidationException(f"Tried to read asset name from spec, " + f"but it includes a template tag: {name}") return name @property @@ -439,7 +440,8 @@ class AssetConfig(Config): if self.auto_version: version = None else: - raise ValidationException(f"Tried to read asset version from spec, but it includes a template tag: {version}") + raise ValidationException(f"Tried to read asset version from spec, " + f"but it includes a template tag: {version}") return str(version) if version is not None else None @property diff --git a/scripts/azureml-assets/azureml/assets/copy_unreleased_assets.py b/scripts/azureml-assets/azureml/assets/copy_unreleased_assets.py index 90713d669..10113fc7e 100644 --- a/scripts/azureml-assets/azureml/assets/copy_unreleased_assets.py +++ b/scripts/azureml-assets/azureml/assets/copy_unreleased_assets.py @@ -49,9 +49,12 @@ def copy_unreleased_assets(release_directory_root: Path, if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-r", "--release-directory", required=True, type=Path, help="Directory to which the release branch has been cloned") - parser.add_argument("-o", "--output-directory", required=True, type=Path, help="Directory to which unreleased assets will be written") - parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, help="Asset config file name to search for") + parser.add_argument("-r", "--release-directory", required=True, type=Path, + help="Directory to which the release branch has been cloned") + parser.add_argument("-o", "--output-directory", required=True, type=Path, + help="Directory to which unreleased assets will be written") + parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, + help="Asset config file name to search for") args = parser.parse_args() # Release assets diff --git a/scripts/azureml-assets/azureml/assets/environment/build.py b/scripts/azureml-assets/azureml/assets/environment/build.py index 7d72ace40..eee147847 100644 --- a/scripts/azureml-assets/azureml/assets/environment/build.py +++ b/scripts/azureml-assets/azureml/assets/environment/build.py @@ -139,12 +139,14 @@ def build_images(input_dirs: List[Path], with ThreadPoolExecutor(max_parallel) as pool: # Find environments under image root directories futures = [] - for asset_config in util.find_assets(input_dirs, asset_config_filename, assets.AssetType.ENVIRONMENT, changed_files): + for asset_config in util.find_assets(input_dirs, asset_config_filename, assets.AssetType.ENVIRONMENT, + changed_files): env_config = asset_config.environment_config_as_object() # Filter by OS if os_to_build and env_config.os.value != os_to_build: - logger.print(f"Skipping build of image for {asset_config.name}: Operating system {env_config.os.value} != {os_to_build}") + logger.print(f"Skipping build of image for {asset_config.name}: " + f"Operating system {env_config.os.value} != {os_to_build}") continue # Pin versions @@ -162,7 +164,8 @@ def build_images(input_dirs: List[Path], # Copy file to output directory without building if output_directory: - util.copy_asset_to_output_dir(asset_config=asset_config, output_directory=output_directory, add_subdir=True) + util.copy_asset_to_output_dir(asset_config=asset_config, output_directory=output_directory, + add_subdir=True) continue # Tag with version from spec @@ -185,13 +188,15 @@ def build_images(input_dirs: List[Path], logger.print(output) logger.end_group() if return_code != 0: - logger.log_error(f"Build of image for {asset_config.name} failed with exit status {return_code}", "Build failure") + logger.log_error(f"Build of image for {asset_config.name} failed with exit status {return_code}", + "Build failure") counters[FAILED_COUNT] += 1 else: logger.log_debug(f"Successfully built image for {asset_config.name}") counters[SUCCESS_COUNT] += 1 if output_directory: - util.copy_asset_to_output_dir(asset_config=asset_config, output_directory=output_directory, add_subdir=True) + util.copy_asset_to_output_dir(asset_config=asset_config, output_directory=output_directory, + add_subdir=True) # Set variables for counter_name in COUNTERS: @@ -206,19 +211,32 @@ def build_images(input_dirs: List[Path], if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-dirs", required=True, help="Comma-separated list of directories containing environments to build") - parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, help="Asset config file name to search for") - parser.add_argument("-o", "--output-directory", type=Path, help="Directory to which successfully built environments will be written") - parser.add_argument("-l", "--build-logs-dir", required=True, type=Path, help="Directory to receive build logs") - parser.add_argument("-p", "--max-parallel", type=int, default=25, help="Maximum number of images to build at the same time") - parser.add_argument("-c", "--changed-files", help="Comma-separated list of changed files, used to filter images") - parser.add_argument("-O", "--os-to-build", choices=[i.value for i in list(assets.Os)], help="Only build environments based on this OS") - parser.add_argument("-r", "--registry", help="Container registry on which to build images") - parser.add_argument("-g", "--resource-group", help="Resource group containing the container registry") - parser.add_argument("-P", "--pin-versions", action="store_true", help="Pin images/packages to latest versions") - parser.add_argument("-t", "--tag-with-version", action="store_true", help="Tag image names using the version in the asset's spec file") - parser.add_argument("-T", "--test-command", help="If building on ACR, command used to test image, relative to build context root") - parser.add_argument("-u", "--push", action="store_true", help="If building on ACR, push after building and (optionally) testing") + parser.add_argument("-i", "--input-dirs", required=True, + help="Comma-separated list of directories containing environments to build") + parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, + help="Asset config file name to search for") + parser.add_argument("-o", "--output-directory", type=Path, + help="Directory to which successfully built environments will be written") + parser.add_argument("-l", "--build-logs-dir", required=True, type=Path, + help="Directory to receive build logs") + parser.add_argument("-p", "--max-parallel", type=int, default=25, + help="Maximum number of images to build at the same time") + parser.add_argument("-c", "--changed-files", + help="Comma-separated list of changed files, used to filter images") + parser.add_argument("-O", "--os-to-build", choices=[i.value for i in list(assets.Os)], + help="Only build environments based on this OS") + parser.add_argument("-r", "--registry", + help="Container registry on which to build images") + parser.add_argument("-g", "--resource-group", + help="Resource group containing the container registry") + parser.add_argument("-P", "--pin-versions", action="store_true", + help="Pin images/packages to latest versions") + parser.add_argument("-t", "--tag-with-version", action="store_true", + help="Tag image names using the version in the asset's spec file") + parser.add_argument("-T", "--test-command", + help="If building on ACR, command used to test image, relative to build context root") + parser.add_argument("-u", "--push", action="store_true", + help="If building on ACR, push after building and (optionally) testing") args = parser.parse_args() # Ensure dependent args are present diff --git a/scripts/azureml-assets/azureml/assets/environment/create_environments.py b/scripts/azureml-assets/azureml/assets/environment/create_environments.py deleted file mode 100644 index ce4e8f682..000000000 --- a/scripts/azureml-assets/azureml/assets/environment/create_environments.py +++ /dev/null @@ -1,99 +0,0 @@ -# --------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# --------------------------------------------------------- - -import argparse -import json -import requests -from pathlib import Path -from urllib.parse import urljoin - -import azureml.assets.util as util -from azureml.assets.util import logger - -ENV_URL_TEMPLATE = "environment/1.0/environments/{environment}/versions/{version}/registry/{registry}" - - -def create_environment(base_url: str, - name: str, - version: str, - registry: str, - image_name: str, - access_token: str, - env_def_with_metadata: object): - url = urljoin(base_url, ENV_URL_TEMPLATE.format(environment=name, version=version, registry=registry)) - headers = { - 'Authorization': f"Bearer {access_token}", - 'Content-Type': "application/json" - } - params = {'imageName': image_name} - response = requests.put(url, headers=headers, json=env_def_with_metadata, params=params) - if not response.ok: - logger.log_error(f"Failed to create {name} version {version}: {response.status_code} {response.text}") - return response.ok - - -def create_environments(deployment_config_file_path: Path, - base_url: str, - registry: str, - access_token: str, - version_template: str = None, - tag_template: str = None): - # Load config - base_path = deployment_config_file_path.parent - with open(deployment_config_file_path) as f: - deployment_config = json.loads(f.read()) - - # Iterate over environments - errors = False - for name, values in deployment_config.items(): - logger.print(f"Creating environment {name} in {registry}") - - # Coerce values into a list - values_list = values if isinstance(values, list) else [values] - - # Iterate over versions, although there's likely just one - for value in values_list: - version = value['version'] - - # Load EnvironmentDefinitionWithSetMetadataDto - with open(base_path / value['path']) as f: - env_def_with_metadata = json.loads(f.read()) - - # Apply tag template to image name - full_image_name = util.apply_tag_template(value['publish']['fullImageName'], tag_template) - - # Apply version template - version = util.apply_version_template(version, version_template) - - # Create environment - success = create_environment(base_url=base_url, name=name, version=version, registry=registry, - image_name=full_image_name, access_token=access_token, - env_def_with_metadata=env_def_with_metadata) - if not success: - errors = True - - # Final messages - if errors: - raise Exception("Errors occurred while creating environments") - - -if __name__ == "__main__": - # Handle command-line args - parser = argparse.ArgumentParser() - parser.add_argument("-i", "--deployment-config", required=True, type=Path, help="Path to deployment config file") - parser.add_argument("-u", "--url", required=True, help="Base AzureML URL, example: https://master.api.azureml-test.ms") - parser.add_argument("-r", "--registry", required=True, help="Name of the target registry") - parser.add_argument("-t", "--token", required=True, help="Access token to use for bearer authentication") - parser.add_argument("-v", "--version-template", help="Template to apply to the version from the deployment config " - "file, example: '{version}.dev1'") - parser.add_argument("-T", "--tag-template", help="Template to apply to fullImageName tags from the deployment " - "config file, example: '{tag}.dev1'") - args = parser.parse_args() - - create_environments(deployment_config_file_path=args.deployment_config, - base_url=args.url, - registry=args.registry, - access_token=args.token, - version_template=args.version_template, - tag_template=args.tag_template) diff --git a/scripts/azureml-assets/azureml/assets/environment/pin_image_versions.py b/scripts/azureml-assets/azureml/assets/environment/pin_image_versions.py index 0a169f082..e0300e103 100644 --- a/scripts/azureml-assets/azureml/assets/environment/pin_image_versions.py +++ b/scripts/azureml-assets/azureml/assets/environment/pin_image_versions.py @@ -126,8 +126,10 @@ def transform_file(input_file: Path, output_file: Path = None): if __name__ == "__main__": parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input", type=Path, help="File containing images to pin to latest versions", required=True) - parser.add_argument("-o", "--output", type=Path, help="File to which output will be written. Defaults to the input file if not specified.") + parser.add_argument("-i", "--input", type=Path, + help="File containing images to pin to latest versions", required=True) + parser.add_argument("-o", "--output", type=Path, + help="File to which output will be written. Defaults to the input file.") args = parser.parse_args() output = args.output or args.input diff --git a/scripts/azureml-assets/azureml/assets/environment/pin_package_versions.py b/scripts/azureml-assets/azureml/assets/environment/pin_package_versions.py index 481f72c28..3d49fd48b 100644 --- a/scripts/azureml-assets/azureml/assets/environment/pin_package_versions.py +++ b/scripts/azureml-assets/azureml/assets/environment/pin_package_versions.py @@ -103,8 +103,10 @@ def transform_file(input_file: Path, output_file: Path = None): if __name__ == "__main__": parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input", type=Path, help="File containing packages to pin to latest versions", required=True) - parser.add_argument("-o", "--output", type=Path, help="File to which output will be written. Defaults to the input file if not specified.") + parser.add_argument("-i", "--input", type=Path, + help="File containing packages to pin to latest versions", required=True) + parser.add_argument("-o", "--output", type=Path, + help="File to which output will be written. Defaults to the input file.") args = parser.parse_args() output = args.output or args.input diff --git a/scripts/azureml-assets/azureml/assets/environment/pin_versions.py b/scripts/azureml-assets/azureml/assets/environment/pin_versions.py index 5fcd1ba4f..51cda9aea 100644 --- a/scripts/azureml-assets/azureml/assets/environment/pin_versions.py +++ b/scripts/azureml-assets/azureml/assets/environment/pin_versions.py @@ -30,8 +30,10 @@ def transform_file(input_file: Path, output_file: Path = None): if __name__ == "__main__": parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input", type=Path, help="File containing images/packages to pin to latest versions", required=True) - parser.add_argument("-o", "--output", type=Path, help="File to which output will be written. Defaults to the input file if not specified.") + parser.add_argument("-i", "--input", type=Path, + help="File containing images/packages to pin to latest versions", required=True) + parser.add_argument("-o", "--output", type=Path, + help="File to which output will be written. Defaults to the input file.") args = parser.parse_args() output = args.output or args.input diff --git a/scripts/azureml-assets/azureml/assets/environment/push.py b/scripts/azureml-assets/azureml/assets/environment/push.py deleted file mode 100644 index d521a0ffe..000000000 --- a/scripts/azureml-assets/azureml/assets/environment/push.py +++ /dev/null @@ -1,86 +0,0 @@ -# --------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# --------------------------------------------------------- - -import argparse -import sys -from concurrent.futures import as_completed, ThreadPoolExecutor -from datetime import datetime, timedelta, timezone -from subprocess import run, PIPE, STDOUT -from timeit import default_timer as timer -from typing import List - -from azureml.assets.util import logger - -TAG_DATETIME_FORMAT = "%Y%m%d%H%M%S" - - -def tag_image(image_name: str, target_image_name: str): - p = run(["docker", "tag", image_name, target_image_name], - stdout=PIPE, - stderr=STDOUT) - return (p.returncode, p.stdout.decode()) - - -def push_image(image_name: str, all_tags: bool = False): - # Create args - args = ["docker", "push"] - if all_tags: - args.append("--all-tags") - args.append(image_name) - - # Push - logger.print(f"Pushing {image_name}") - start = timer() - p = run(args, - stdout=PIPE, - stderr=STDOUT) - end = timer() - logger.print(f"{image_name} pushed in {timedelta(seconds=end-start)}") - return (image_name, p.returncode, p.stdout.decode()) - - -def push_images(image_names: List[str], target_image_prefix: str, tags: List[str], max_parallel: int): - with ThreadPoolExecutor(max_parallel) as pool: - futures = [] - for image_name in image_names: - # Tag image - target_image_base_name = f"{target_image_prefix}{image_name}" - logger.print(f"Tagging {target_image_base_name} as {tags}") - for tag in tags: - target_image_name = f"{target_image_base_name}:{tag}" - (return_code, output) = tag_image(image_name, target_image_name) - if return_code != 0: - logger.log_error(f"Failed to tag {image_name} as {target_image_name}: {output}", "Tag failure") - sys.exit(1) - - # Start push - futures.append(pool.submit(push_image, target_image_base_name, True)) - - for future in as_completed(futures): - (image_name, return_code, output) = future.result() - logger.start_group(f"{image_name} push log") - logger.print(output) - logger.end_group() - if return_code != 0: - logger.log_error(f"Push of {image_name} failed with exit status {return_code}", "Push failure") - sys.exit(1) - - -if __name__ == '__main__': - # Handle command-line args - parser = argparse.ArgumentParser() - parser.add_argument("-i", "--image-names", required=True, help="Comma-separated list of image names to push") - parser.add_argument("-t", "--target-image-prefix", required=True, help="Prefix to use when tagging images") - parser.add_argument("-p", "--max-parallel", type=int, default=5, help="Maximum number of images to push at the same time") - args = parser.parse_args() - - # Convert comma-separated values to lists - image_names = args.image_names.split(",") if args.image_names else [] - - # Generate tags - timestamp_tag = datetime.now(timezone.utc).strftime(TAG_DATETIME_FORMAT) - tags = [timestamp_tag, "latest"] - - # Push images - push_images(image_names, args.target_image_prefix, tags, args.max_parallel) diff --git a/scripts/azureml-assets/azureml/assets/environment/test_images.py b/scripts/azureml-assets/azureml/assets/environment/test_images.py deleted file mode 100644 index c891ed4e3..000000000 --- a/scripts/azureml-assets/azureml/assets/environment/test_images.py +++ /dev/null @@ -1,92 +0,0 @@ -# --------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# --------------------------------------------------------- - -import argparse -import sys -from collections import Counter -from datetime import timedelta -from pathlib import Path -from subprocess import run, PIPE, STDOUT -from timeit import default_timer as timer -from typing import List, Tuple - -import azureml.assets as assets -import azureml.assets.util as util -from azureml.assets.util import logger - -SUCCESS_COUNT = "success_count" -FAILED_COUNT = "failed_count" -COUNTERS = [SUCCESS_COUNT, FAILED_COUNT] -TEST_PHRASE = "hello world!" - - -def test_image(asset_config: assets.AssetConfig, image_name: str) -> Tuple[int, str]: - logger.print(f"Testing image for {asset_config.name}") - start = timer() - p = run(["docker", "run", "--entrypoint", "python", image_name, "-c", f"print(\"{TEST_PHRASE}\")"], - stdout=PIPE, - stderr=STDOUT) - end = timer() - logger.print(f"Image for {asset_config.name} tested in {timedelta(seconds=end-start)}") - return (p.returncode, p.stdout.decode()) - - -def test_images(input_dirs: List[Path], - asset_config_filename: str, - output_directory: Path, - os_to_test: str = None) -> bool: - counters = Counter() - for asset_config in util.find_assets(input_dirs, asset_config_filename, assets.AssetType.ENVIRONMENT): - env_config = asset_config.environment_config_as_object() - - # Filter by OS - if os_to_test and env_config.os.value != os_to_test: - logger.print(f"Not testing image for {asset_config.name}: Operating system {env_config.os.value} != {os_to_test}") - continue - - # Skip images without build context - if not env_config.build_enabled: - logger.print(f"Not testing image for {asset_config.name}: No build context specified") - continue - - # Test image - return_code, output = test_image(asset_config, env_config.image_name) - if return_code != 0 or not output.startswith(TEST_PHRASE): - logger.log_error(f"Testing of image for {asset_config.name} failed: {output}", title="Testing failure") - counters[FAILED_COUNT] += 1 - else: - logger.log_debug(f"Successfully tested image for {asset_config.name}") - counters[SUCCESS_COUNT] += 1 - if output_directory: - util.copy_asset_to_output_dir(asset_config=asset_config, output_dir=output_directory, add_subdir=True) - - # Set variables - for counter_name in COUNTERS: - logger.set_output(counter_name, counters[counter_name]) - - if counters[FAILED_COUNT] > 0: - logger.log_error(f"{counters[FAILED_COUNT]} environment image(s) failed to test") - return False - return True - - -if __name__ == '__main__': - # Handle command-line args - parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-dirs", required=True, help="Comma-separated list of directories containing environments to test") - parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, help="Asset config file name to search for") - parser.add_argument("-o", "--output-directory", type=Path, help="Directory to which successfully tested environments will be written") - parser.add_argument("-O", "--os-to-test", choices=[i.value for i in list(assets.Os)], help="Only test environments based on this OS") - args = parser.parse_args() - - # Convert comma-separated values to lists - input_dirs = [Path(d) for d in args.input_dirs.split(",")] - - # Test images - success = test_images(input_dirs=input_dirs, - asset_config_filename=args.asset_config_filename, - output_directory=args.output_directory, - os_to_test=args.os_to_test) - if not success: - sys.exit(1) diff --git a/scripts/azureml-assets/azureml/assets/release/asset_publish.py b/scripts/azureml-assets/azureml/assets/release/asset_publish.py index 1bf0ca7e0..21f907a51 100644 --- a/scripts/azureml-assets/azureml/assets/release/asset_publish.py +++ b/scripts/azureml-assets/azureml/assets/release/asset_publish.py @@ -81,7 +81,8 @@ if __name__ == '__main__': if registry_name != "azureml": final_version = final_version + '-' + component_version_with_buildId print("final version: "+final_version) - cmd = f"az ml component create --file {spec_path} --registry-name {registry_name} --version {final_version} --workspace {workspace} --resource-group {resource_group}" + cmd = f"az ml component create --file {spec_path} --registry-name {registry_name} --version {final_version} " \ + f"--workspace {workspace} --resource-group {resource_group}" print(cmd) try: check_call(cmd, shell=True) diff --git a/scripts/azureml-assets/azureml/assets/release/e2e_test.py b/scripts/azureml-assets/azureml/assets/release/e2e_test.py index dd091a8c1..483064b45 100644 --- a/scripts/azureml-assets/azureml/assets/release/e2e_test.py +++ b/scripts/azureml-assets/azureml/assets/release/e2e_test.py @@ -52,7 +52,8 @@ if __name__ == '__main__': data = yaml.load(fp, Loader=yaml.FullLoader) for test_group in data: print(f"now processing test group: {test_group}") - p = run(f"python3 -u group_test.py -i {area} -g {test_group} -s {subscription_id} -r {resource_group} -w {workspace}", shell=True) + p = run(f"python3 -u group_test.py -i {area} -g {test_group} -s {subscription_id} -r {resource_group} " + f"-w {workspace}", shell=True) return_code = p.returncode print(return_code) final_report[area.name].append(f"test group {test_group} returned {return_code}") diff --git a/scripts/azureml-assets/azureml/assets/release/test_file_convert.py b/scripts/azureml-assets/azureml/assets/release/test_file_convert.py index 03caaa46e..df9fa8832 100644 --- a/scripts/azureml-assets/azureml/assets/release/test_file_convert.py +++ b/scripts/azureml-assets/azureml/assets/release/test_file_convert.py @@ -18,9 +18,12 @@ def copy_replace_dir(source: Path, dest: Path): if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-dir", required=True, type=Path, help="dir path of tests.yml") - parser.add_argument("-a", "--test-area", required=True, type=str, help="the test area name") - parser.add_argument("-r", "--release-directory", required=True, type=Path, help="Directory to which the release branch has been cloned") + parser.add_argument("-i", "--input-dir", required=True, type=Path, + help="dir path of tests.yml") + parser.add_argument("-a", "--test-area", required=True, type=str, + help="the test area name") + parser.add_argument("-r", "--release-directory", required=True, type=Path, + help="Directory to which the release branch has been cloned") args = parser.parse_args() yaml_name = "tests.yml" tests_folder = args.release_directory / "tests" / args.test_area diff --git a/scripts/azureml-assets/azureml/assets/tag_released_assets.py b/scripts/azureml-assets/azureml/assets/tag_released_assets.py index c7e2d6bdd..b4aea70f2 100644 --- a/scripts/azureml-assets/azureml/assets/tag_released_assets.py +++ b/scripts/azureml-assets/azureml/assets/tag_released_assets.py @@ -42,9 +42,12 @@ def tag_released_assets(input_directory: Path, if __name__ == "__main__": # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-directory", required=True, type=Path, help="Directory containing released assets") - parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, help="Asset config file name to search for") - parser.add_argument("-r", "--release-directory", required=True, type=Path, help="Directory to which the release branch has been cloned") + parser.add_argument("-i", "--input-directory", required=True, type=Path, + help="Directory containing released assets") + parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, + help="Asset config file name to search for") + parser.add_argument("-r", "--release-directory", required=True, type=Path, + help="Directory to which the release branch has been cloned") parser.add_argument("-u", "--username", help="Username for git push") parser.add_argument("-e", "--email", help="Email for git push") args = parser.parse_args() diff --git a/scripts/azureml-assets/azureml/assets/test_assets.py b/scripts/azureml-assets/azureml/assets/test_assets.py index 71e2880c2..e685acebc 100644 --- a/scripts/azureml-assets/azureml/assets/test_assets.py +++ b/scripts/azureml-assets/azureml/assets/test_assets.py @@ -105,9 +105,12 @@ def test_assets(input_dirs: List[Path], if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-dirs", required=True, help="Comma-separated list of directories containing environments to test") - parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, help="Asset config file name to search for") - parser.add_argument("-p", "--package-versions-file", required=True, type=Path, help="File with package versions for the base conda environment") + parser.add_argument("-i", "--input-dirs", required=True, + help="Comma-separated list of directories containing environments to test") + parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, + help="Asset config file name to search for") + parser.add_argument("-p", "--package-versions-file", required=True, type=Path, + help="File with package versions for the base conda environment") parser.add_argument("-c", "--changed-files", help="Comma-separated list of changed files, used to filter assets") parser.add_argument("-r", "--reports-dir", type=Path, help="Directory for pytest reports") args = parser.parse_args() diff --git a/scripts/azureml-assets/azureml/assets/update_assets.py b/scripts/azureml-assets/azureml/assets/update_assets.py index 96f405dc0..ef4c7c96c 100644 --- a/scripts/azureml-assets/azureml/assets/update_assets.py +++ b/scripts/azureml-assets/azureml/assets/update_assets.py @@ -171,12 +171,18 @@ def update_assets(input_dirs: List[Path], if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-dirs", required=True, help="Comma-separated list of directories containing assets") - parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, help="Asset config file name to search for") - parser.add_argument("-r", "--release-directory", required=True, type=Path, help="Directory to which the release branch has been cloned") - parser.add_argument("-o", "--output-directory", type=Path, help="Directory to which new/updated assets will be written, defaults to release directory") - parser.add_argument("-c", "--copy-only", action="store_true", help="Just copy assets into the release directory") - parser.add_argument("-s", "--skip-unreleased", action="store_true", help="Skip unreleased dynamically-versioned assets in the release branch") + parser.add_argument("-i", "--input-dirs", required=True, + help="Comma-separated list of directories containing assets") + parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, + help="Asset config file name to search for") + parser.add_argument("-r", "--release-directory", required=True, type=Path, + help="Directory to which the release branch has been cloned") + parser.add_argument("-o", "--output-directory", type=Path, + help="Directory to which new/updated assets will be written, defaults to release directory") + parser.add_argument("-c", "--copy-only", action="store_true", + help="Just copy assets into the release directory") + parser.add_argument("-s", "--skip-unreleased", action="store_true", + help="Skip unreleased dynamically-versioned assets in the release branch") args = parser.parse_args() # Convert comma-separated values to lists diff --git a/scripts/azureml-assets/azureml/assets/update_spec.py b/scripts/azureml-assets/azureml/assets/update_spec.py index 66af522dc..b7be89975 100644 --- a/scripts/azureml-assets/azureml/assets/update_spec.py +++ b/scripts/azureml-assets/azureml/assets/update_spec.py @@ -67,9 +67,11 @@ def update(asset_config: assets.AssetConfig, release_directory_root: Path = None Args: asset_config (assets.AssetConfig): AssetConfig object. release_directory_root (Path, optional): Directory to which the release branch has been cloned. - output_file (Path, optional): File to which updated spec file will be written. If unspecified, the original spec file will be updated. + output_file (Path, optional): File to which updated spec file will be written. + If unspecified, the original spec file will be updated. version (str, optional): Version to use instead of the one in the asset config file. - include_commit_hash (bool, optional): Whether to include the commit hash in the data available for template replacemnt. + include_commit_hash (bool, optional): Whether to include the commit hash in the data available for + template replacemnt. data (Dict[str, object], optional): If provided, use this data instead of calling create_template_data(). """ # Reuse or create data @@ -92,9 +94,12 @@ def update(asset_config: assets.AssetConfig, release_directory_root: Path = None if __name__ == "__main__": parser = argparse.ArgumentParser() - parser.add_argument("-a", "--asset-config", required=True, type=Path, help="Asset config file that points to the spec file to update") - parser.add_argument("-r", "--release-directory", type=Path, help="Directory to which the release branch has been cloned") - parser.add_argument("-o", "--output", type=Path, help="File to which output will be written. Defaults to the original spec file if not specified.") + parser.add_argument("-a", "--asset-config", required=True, type=Path, + help="Asset config file that points to the spec file to update") + parser.add_argument("-r", "--release-directory", type=Path, + help="Directory to which the release branch has been cloned") + parser.add_argument("-o", "--output", type=Path, + help="File to which output will be written. Defaults to the original spec file.") args = parser.parse_args() update(asset_config=args.asset_config, release_directory_root=args.release_directory, output_file=args.output) diff --git a/scripts/azureml-assets/azureml/assets/util/util.py b/scripts/azureml-assets/azureml/assets/util/util.py index c9c07f159..37a8432b0 100644 --- a/scripts/azureml-assets/azureml/assets/util/util.py +++ b/scripts/azureml-assets/azureml/assets/util/util.py @@ -15,7 +15,7 @@ RELEASE_SUBDIR = "latest" EXCLUDE_DIR_PREFIX = "!" -# See https://stackoverflow.com/questions/4187564/recursively-compare-two-directories-to-ensure-they-have-the-same-files-and-subdi +# See https://stackoverflow.com/questions/4187564 def are_dir_trees_equal(dir1: Path, dir2: Path, enable_logging: bool = False, ignore_eol: bool = True) -> bool: """Compare two directories recursively based on files names and content. @@ -29,7 +29,6 @@ def are_dir_trees_equal(dir1: Path, dir2: Path, enable_logging: bool = False, ig bool: True if the directory trees are the same and there were no errors while accessing the directories or files, False otherwise. """ - dirs_cmp = filecmp.dircmp(dir1, dir2) if dirs_cmp.left_only: _log_diff(f"Compared {dir1} and {dir2} and found these only in {dir1}: {dirs_cmp.left_only}", enable_logging) @@ -241,8 +240,8 @@ def find_assets(input_dirs: Union[List[Path], Path], Args: input_dirs (Union[List[Path], Path]): Directories to search in. asset_config_filename (str, optional): Asset config filename to search for. - types (Union[List[assets.AssetType], assets.AssetType], optional): AssetTypes to search for. Will not filter if unspecified. - changed_files (List[Path], optional): Changed files, used to filter assets in input_dirs. Will not filter if unspecified. + types (Union[List[assets.AssetType], assets.AssetType], optional): AssetTypes to search for. + changed_files (List[Path], optional): Changed files, used to filter assets in input_dirs. exclude_dirs (Union[List[Path], Path], optional): Directories that should be excluded from the search. Returns: @@ -273,7 +272,7 @@ def find_asset_config_files(input_dirs: Union[List[Path], Path], Args: input_dirs (Union[List[Path], Path]): Directories to search in. asset_config_filename (str): Asset config filename to search for. - changed_files (List[Path], optional): Changed files, used to filter assets in input_dirs. Will not filter if unspecified. + changed_files (List[Path], optional): Changed files, used to filter assets in input_dirs. exclude_dirs (Union[List[Path], Path], optional): Directories that should be excluded from the search. Returns: diff --git a/scripts/azureml-assets/azureml/assets/validate_assets.py b/scripts/azureml-assets/azureml/assets/validate_assets.py index 092b92e26..ab145ccec 100644 --- a/scripts/azureml-assets/azureml/assets/validate_assets.py +++ b/scripts/azureml-assets/azureml/assets/validate_assets.py @@ -79,8 +79,10 @@ def validate_assets(input_dirs: List[Path], if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-dirs", required=True, help="Comma-separated list of directories containing assets") - parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, help="Asset config file name to search for") + parser.add_argument("-i", "--input-dirs", required=True, + help="Comma-separated list of directories containing assets") + parser.add_argument("-a", "--asset-config-filename", default=assets.DEFAULT_ASSET_FILENAME, + help="Asset config file name to search for") args = parser.parse_args() # Convert comma-separated values to lists diff --git a/scripts/azureml-assets/validation_rules.json b/scripts/azureml-assets/validation_rules.json new file mode 100644 index 000000000..fc2610410 --- /dev/null +++ b/scripts/azureml-assets/validation_rules.json @@ -0,0 +1,24 @@ +{ + "pep8": { + "ignore-file": [ + "azureml/assets/__init__.py:F401", + "azureml/assets/environment/__init__.py:F401", + "azureml/assets/util/__init__.py:F401" + ], + "justifications": [ + "__init__.py - namespace level files excluded from validation" + ] + }, + "doc": { + "exclude": [ + "azureml/assets/__init__.py", + "azureml/assets/environment/__init__.py", + "azureml/assets/util/__init__.py", + "." + ], + "justifications": [ + "__init__.py - namespace level files excluded from validation", + ". - not enabled yet" + ] + } +} diff --git a/scripts/flake_rules.json b/scripts/flake_rules.json deleted file mode 100644 index 065c489c9..000000000 --- a/scripts/flake_rules.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "pep8": { - "ignore-file": [ - "azureml-assets/azureml/assets/__init__.py:F401", - "azureml-assets/azureml/assets/environment/__init__.py:F401", - "azureml-assets/azureml/assets/util/__init__.py:F401" - ], - "justifications": [ - "__init__.py - namespace level files excluded from validation" - ] - } -} \ No newline at end of file diff --git a/scripts/script-check-requirements.txt b/scripts/script-check-requirements.txt index 369eff651..54b8cabd6 100644 --- a/scripts/script-check-requirements.txt +++ b/scripts/script-check-requirements.txt @@ -1 +1,2 @@ -flake8==4.0.1 \ No newline at end of file +flake8==4.0.1 +pydocstyle==6.1.1 \ No newline at end of file diff --git a/scripts/validation/code_health.py b/scripts/validation/code_health.py index 701c64c67..a5c4cfe2f 100644 --- a/scripts/validation/code_health.py +++ b/scripts/validation/code_health.py @@ -7,32 +7,74 @@ import json import sys from pathlib import Path from subprocess import run, PIPE, STDOUT -from typing import Dict, List, Tuple +from typing import Dict, List, Set, Tuple -FLAKE_RULES_FILE = "flake_rules.json" -DEFAULT_MAX_LINE_LENGTH = 10000 -IGNORE = "ignore" -IGNORE_FILE = "ignore-file" -EXCLUDE = "exclude" -MAX_LINE_LENGTH = "max-line-length" +RULES_FILENAME = "validation_rules.json" -def run_flake8(testpath: Path, flake_rules: Dict[str, List[str]]) -> Tuple[int, str]: - ignore = flake_rules.get(IGNORE, []) - file_ignore = flake_rules.get(IGNORE_FILE, []) - exclude = flake_rules.get(EXCLUDE, []) - max_line_length = flake_rules.get(MAX_LINE_LENGTH, DEFAULT_MAX_LINE_LENGTH) +class Rules: + ROOT_KEY = "pep8" + IGNORE = "ignore" + IGNORE_FILE = "ignore-file" + EXCLUDE = "exclude" + MAX_LINE_LENGTH = "max-line-length" + DEFAULT_MAX_LINE_LENGTH = 10000 + + def __init__(self, file_name: Path = None): + if file_name is not None and file_name.exists(): + parent_path = file_name.parent + # Load rules from file + with open(file_name) as f: + rules = json.load(f).get(self.ROOT_KEY, {}) + self.ignore = set(rules.get(self.IGNORE, [])) + self.ignore_file = self._parse_ignore_file(parent_path, rules.get(self.IGNORE_FILE, [])) + self.exclude = {parent_path / p for p in rules.get(self.EXCLUDE, [])} + self.max_line_length = rules.get(self.MAX_LINE_LENGTH) + else: + # Initialize empty + self.ignore = set() + self.ignore_file = {} + self.exclude = set() + self.max_line_length = None + + @staticmethod + def _parse_ignore_file(parent_path: Path, ignore_file: List[str]) -> Dict[str, Set[str]]: + results = {} + for pair in ignore_file: + file, file_rules = pair.split(":") + file = parent_path / file + file_rules = {r.strip() for r in file_rules.split(",") if not r.isspace()} + results[file] = file_rules + return results + + def get_effective_max_line_length(self) -> int: + return self.max_line_length or self.DEFAULT_MAX_LINE_LENGTH + + def __or__(self, other: "Rules") -> "Rules": + rules = Rules() + rules.ignore = self.ignore | other.ignore + for key in self.ignore_file.keys() | other.ignore_file.keys(): + rules.ignore_file[key] = self.ignore_file.get(key, set()) | other.ignore_file.get(key, set()) + rules.exclude = self.exclude | other.exclude + rules.max_line_length = other.max_line_length or self.max_line_length + return rules + + +def run_flake8(testpath: Path, rules: Rules) -> Tuple[int, str]: cmd = [ "flake8", - f"--max-line-length={max_line_length}", + f"--max-line-length={rules.get_effective_max_line_length()}", str(testpath) ] - if exclude: - cmd.insert(1, "--exclude={}".format(",".join(exclude))) - if file_ignore: - cmd.insert(1, "--per-file-ignores={}".format(",".join(file_ignore))) - if ignore: - cmd.insert(1, "--ignore={}".format(",".join(ignore))) + if rules.exclude: + cmd.insert(1, "--exclude={}".format(",".join([str(e) for e in rules.exclude]))) + if rules.ignore_file: + file_ignore_list = [] + for file, ignores in rules.ignore_file.items(): + file_ignore_list.extend([f"{file}:{i}" for i in ignores]) + cmd.insert(1, "--per-file-ignores={}".format(",".join(file_ignore_list))) + if rules.ignore: + cmd.insert(1, "--ignore={}".format(",".join(rules.ignore))) print(f"Running {cmd}") p = run(cmd, @@ -42,68 +84,35 @@ def run_flake8(testpath: Path, flake_rules: Dict[str, List[str]]) -> Tuple[int, return p.stdout.decode() -def load_rules(testpath: Path) -> Dict[str, List[str]]: - flake_rules_file = testpath / FLAKE_RULES_FILE - flake_rules = {} - if flake_rules_file.exists(): - with open(flake_rules_file) as f: - flake_rules = json.load(f).get('pep8', {}) - - # Handle relative paths - if IGNORE_FILE in flake_rules: - file_ignore = [] - for pair in flake_rules[IGNORE_FILE]: - file, rules = pair.split(":") - file_resolved = str(testpath / file) - file_ignore.append(f"{file_resolved}:{rules}") - flake_rules[IGNORE_FILE] = file_ignore - if EXCLUDE in flake_rules: - flake_rules[EXCLUDE] = [str(testpath / p) for p in flake_rules[EXCLUDE]] - - return flake_rules - - -def combine_rules(rule_a: Dict[str, List[str]], rule_b: Dict[str, List[str]]) -> Dict[str, List[str]]: - rule = {} - rule[IGNORE] = rule_a.get(IGNORE, []) + rule_b.get(IGNORE, []) - rule[IGNORE_FILE] = rule_a.get(IGNORE_FILE, []) + rule_b.get(IGNORE_FILE, []) - rule[EXCLUDE] = rule_a.get(EXCLUDE, []) + rule_b.get(EXCLUDE, []) - rule[MAX_LINE_LENGTH] = min(rule_a.get(MAX_LINE_LENGTH, DEFAULT_MAX_LINE_LENGTH), - rule_b.get(MAX_LINE_LENGTH, DEFAULT_MAX_LINE_LENGTH)) - - return rule - - -def inherit_flake_rules(rootpath: Path, testpath: Path) -> Dict[str, List[str]]: - flake_rules = {} - upperpath = testpath - while upperpath != rootpath: - upperpath = upperpath.parent - flake_rules = combine_rules(flake_rules, load_rules(upperpath)) - return flake_rules +def inherit_rules(rootpath: Path, testpath: Path) -> Rules: + # Process paths from rootpath to testpath, to ensure max_line_length is calculated properly + paths = [p for p in testpath.parents if p == rootpath or p.is_relative_to(rootpath)] + paths.reverse() + rules = Rules() + for path in paths: + rules |= Rules(path / RULES_FILENAME) + return rules def test(rootpath: Path, testpath: Path) -> bool: - test_path_flake_rules = inherit_flake_rules(rootpath, testpath) + testpath_rules = inherit_rules(rootpath, testpath) - flake_rules_files = list(testpath.rglob(FLAKE_RULES_FILE)) - dirs = [p.parent for p in flake_rules_files] + rules_files = list(testpath.rglob(RULES_FILENAME)) + dirs = [p.parent for p in rules_files] - if not testpath == dirs: + if testpath not in dirs: dirs.insert(0, testpath) - output = [] + errors = [] for path in dirs: - flake_rules = {} - flake_rules[EXCLUDE] = [str(f.parent) for f in flake_rules_files if f.parent != path] - inherited_rules = inherit_flake_rules(testpath, path) - flake_rules = combine_rules(combine_rules(flake_rules, test_path_flake_rules), - combine_rules(inherited_rules, load_rules(path))) - output.extend([line for line in run_flake8(path, flake_rules).split("\n") if len(line) > 0]) + rules = Rules() + rules.exclude = {d for d in dirs if d != path and not path.is_relative_to(d)} + rules |= testpath_rules | inherit_rules(testpath, path) | Rules(path / RULES_FILENAME) + errors.extend([line for line in run_flake8(path, rules).splitlines() if len(line) > 0]) - if len(output) > 0: + if len(errors) > 0: print("flake8 errors:") - for line in output: + for line in errors: print(line) return False return True @@ -112,8 +121,10 @@ def test(rootpath: Path, testpath: Path) -> bool: if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-directory", required=True, type=Path, help="Directory to validate") - parser.add_argument("-r", "--root-directory", type=Path, help="Root directory containing flake8 rules, must be a parent of --input-directory") + parser.add_argument("-i", "--input-directory", required=True, type=Path, + help="Directory to validate") + parser.add_argument("-r", "--root-directory", type=Path, + help="Root directory containing flake8 rules, must be a parent of --input-directory") args = parser.parse_args() # Handle directories diff --git a/scripts/validation/copyright_validation.py b/scripts/validation/copyright_validation.py index 3ce09366b..767e187eb 100644 --- a/scripts/validation/copyright_validation.py +++ b/scripts/validation/copyright_validation.py @@ -40,8 +40,10 @@ def test(testpaths: List[Path], excludes: List[Path] = []) -> bool: if __name__ == '__main__': # Handle command-line args parser = argparse.ArgumentParser() - parser.add_argument("-i", "--input-directories", required=True, type=Path, nargs='+', help="Directories to validate") - parser.add_argument("-e", "--excludes", default=[], type=Path, nargs='+', help="Directories to exclude") + parser.add_argument("-i", "--input-directories", required=True, type=Path, nargs='+', + help="Directories to validate") + parser.add_argument("-e", "--excludes", default=[], type=Path, nargs='+', + help="Directories to exclude") args = parser.parse_args() success = test(args.input_directories, args.excludes) diff --git a/scripts/validation/doc_style.py b/scripts/validation/doc_style.py new file mode 100644 index 000000000..1d4ea1239 --- /dev/null +++ b/scripts/validation/doc_style.py @@ -0,0 +1,149 @@ +# --------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# --------------------------------------------------------- + +import argparse +import json +import re +import sys +from pathlib import Path +from subprocess import run, PIPE, STDOUT +from typing import List, Set + +RULES_FILENAME = "validation_rules.json" +FILE_NAME_PATTERN = re.compile(r"^(.+):\d+\s+") + + +class Rules: + ROOT_KEY = "doc" + IGNORE = "ignore" + EXCLUDE = "exclude" + FORCE = "force" + + def __init__(self, file_name: Path = None): + if file_name is not None and file_name.exists(): + # Load rules from file + with open(file_name) as f: + rules = json.load(f).get(self.ROOT_KEY, {}) + self.ignore = set(rules.get(self.IGNORE, [])) + self.exclude = {file_name.parent / p for p in rules.get(self.EXCLUDE, [])} + self.force = set(rules.get(self.FORCE, [])) + else: + # Initialize empty + self.ignore = set() + self.exclude = set() + self.force = set() + + def __or__(self, other: "Rules") -> "Rules": + rules = Rules() + rules.ignore = self.ignore | other.ignore + rules.exclude = self.exclude | other.exclude + rules.force = self.force | other.force + return rules + + +def run_docstyle(testpath: Path, rules: Rules): + cmd = [ + "pydocstyle", + r"--match=.*\.py", + str(testpath) + ] + + ignore = rules.ignore - rules.force + if ignore: + cmd.insert(1, "--ignore={}".format(",".join(ignore))) + + print(f"Running {cmd}") + p = run(cmd, + stdout=PIPE, + stderr=STDOUT) + + return p.stdout.decode() + + +def filter_docstyle_output(output: str, rules: Rules) -> List[str]: + lines = [line for line in output.splitlines() if len(line) > 0] + if lines: + filtered_lines = [] + for i in range(0, len(lines) - 1, 2): + line = lines[i] + match = FILE_NAME_PATTERN.match(line) + if not match: + raise Exception(f"Unable to extract filename from {line}") + file = Path(match.group(1)) + file_is_excluded = False + for exclude in rules.exclude: + if file == exclude or (exclude.is_dir() and file.is_relative_to(exclude)): + file_is_excluded = True + break + if not file_is_excluded: + filtered_lines.append(line) + filtered_lines.append(lines[i + 1]) + lines = filtered_lines + + return lines + + +def inherit_rules(rootpath: Path, testpath: Path) -> Rules: + rules = Rules() + upperpath = testpath + while upperpath != rootpath: + upperpath = upperpath.parent + rules |= Rules(upperpath / RULES_FILENAME) + return rules + + +def test(rootpath: Path, testpath: Path, force: Set[str] = {}) -> bool: + testpath_rules = inherit_rules(rootpath, testpath) + + rules_files = list(testpath.rglob(RULES_FILENAME)) + dirs = [p.parent for p in rules_files] + + if testpath not in dirs: + dirs.insert(0, testpath) + + errors = [] + for path in dirs: + rules = Rules() + rules.exclude = {d for d in dirs if d != path and not path.is_relative_to(d)} + rules.force = force + rules |= testpath_rules | inherit_rules(testpath, path) | Rules(path / RULES_FILENAME) + + output = run_docstyle(path, rules) + filtered_output = filter_docstyle_output(output, rules) + errors.extend(filtered_output) + + if len(errors) > 0: + print("pydocstyle errors:") + for line in errors: + print(line) + return False + return True + + +if __name__ == '__main__': + # Handle command-line args + parser = argparse.ArgumentParser() + parser.add_argument("-i", "--input-directory", required=True, type=Path, + help="Directory to validate") + parser.add_argument("-r", "--root-directory", type=Path, + help="Root directory containing docstyle rules, must be a parent of --input-directory") + parser.add_argument("-f", "--force", default="", + help="Comma-separated list of rules that can't be ignored") + args = parser.parse_args() + + # Handle directories + input_directory = args.input_directory + root_directory = args.root_directory + if root_directory is None: + root_directory = args.input_directory + elif not input_directory.is_relative_to(root_directory): + parser.error(f"{root_directory} is not a parent directory of {input_directory}") + + # Parse forced rules + force = {r.strip() for r in args.force.split(",") if not r.isspace()} + + success = test(root_directory, input_directory, force) + + if not success: + sys.exit(1) diff --git a/scripts/validation/validation_rules.json b/scripts/validation/validation_rules.json new file mode 100644 index 000000000..c63b2b207 --- /dev/null +++ b/scripts/validation/validation_rules.json @@ -0,0 +1,10 @@ +{ + "doc": { + "exclude": [ + "." + ], + "justifications": [ + ". - not enabled yet" + ] + } +} diff --git a/test/test_update_assets.py b/test/test_update_assets.py index c309ad32b..b50231b43 100644 --- a/test/test_update_assets.py +++ b/test/test_update_assets.py @@ -31,7 +31,9 @@ def test_validate_assets(test_subdir: str, create_tag: bool): expected_dir = test_dir / "expected" # Temp directory helps keep the original release directory clean - with tempfile.TemporaryDirectory(prefix="release-") as temp_dir1, tempfile.TemporaryDirectory(prefix="output-") as temp_dir2, tempfile.TemporaryDirectory(prefix="expected-") as temp_dir3: + with tempfile.TemporaryDirectory(prefix="release-") as temp_dir1, \ + tempfile.TemporaryDirectory(prefix="output-") as temp_dir2, \ + tempfile.TemporaryDirectory(prefix="expected-") as temp_dir3: temp_release_path = Path(temp_dir1) temp_output_path = Path(temp_dir2) temp_expected_path = Path(temp_dir3) diff --git a/test/validation_rules.json b/test/validation_rules.json new file mode 100644 index 000000000..c63b2b207 --- /dev/null +++ b/test/validation_rules.json @@ -0,0 +1,10 @@ +{ + "doc": { + "exclude": [ + "." + ], + "justifications": [ + ". - not enabled yet" + ] + } +} diff --git a/training/validation_rules.json b/training/validation_rules.json new file mode 100644 index 000000000..c63b2b207 --- /dev/null +++ b/training/validation_rules.json @@ -0,0 +1,10 @@ +{ + "doc": { + "exclude": [ + "." + ], + "justifications": [ + ". - not enabled yet" + ] + } +} diff --git a/validation_rules.json b/validation_rules.json new file mode 100644 index 000000000..32e643497 --- /dev/null +++ b/validation_rules.json @@ -0,0 +1,5 @@ +{ + "pep8": { + "max-line-length": 119 + } +}