Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

{CI} Fix static analysis #6509

Merged
merged 35 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
0f6b778
Update azure-pipelines.yml
wangzelin007 Jul 12, 2023
aa2962e
Update azure-pipelines.yml
wangzelin007 Jul 12, 2023
5d49c19
Update azure-pipelines.yml
wangzelin007 Jul 12, 2023
03b4199
Update azure-pipelines.yml
wangzelin007 Jul 12, 2023
cbf7b3d
Update azure-pipelines.yml
wangzelin007 Aug 3, 2023
8a7f42b
update
wangzelin007 Aug 3, 2023
46ae8fb
Empty
wangzelin007 Aug 9, 2023
9231cd7
Merge branch 'main' into fix_static_analysis
wangzelin007 Aug 11, 2023
158dec1
Update azure-pipelines.yml
wangzelin007 Aug 11, 2023
2613175
Update azdev_setup.yml
wangzelin007 Aug 11, 2023
3568713
Update azure-pipelines.yml
wangzelin007 Aug 11, 2023
377f4f0
Update azure-pipelines.yml
wangzelin007 Aug 11, 2023
bf31b04
Empty
wangzelin007 Nov 23, 2023
125df2c
Merge branch 'main' into fix_static_analysis
wangzelin007 Nov 23, 2023
d0055e4
update
wangzelin007 Nov 23, 2023
6a2cec8
modify containerapp
wangzelin007 Nov 23, 2023
d4f8312
update
wangzelin007 Nov 23, 2023
f095fc8
Update azdev_style_check.py
wangzelin007 Nov 23, 2023
c69cc0e
Update azdev_style_check.py
wangzelin007 Nov 23, 2023
b15bde6
Update azdev_style_check.py
wangzelin007 Nov 23, 2023
38a0f8a
update
wangzelin007 Nov 23, 2023
06f48fc
update
wangzelin007 Dec 1, 2023
d5da72a
Delete azdev_style_check.py
wangzelin007 Dec 1, 2023
65c43a0
Update test_source.py
wangzelin007 Dec 1, 2023
112e685
Update azdev_linter_style.py
wangzelin007 Dec 7, 2023
9f9d594
Update __init__.py
wangzelin007 Dec 7, 2023
537f836
todo
wangzelin007 Dec 12, 2023
f05925d
update
wangzelin007 Dec 12, 2023
a373c6c
update
wangzelin007 Dec 12, 2023
e48d1a3
Update __init__.py
wangzelin007 Dec 13, 2023
c670b23
Update azure-pipelines.yml
wangzelin007 Dec 13, 2023
71b9b3e
update
wangzelin007 Dec 13, 2023
8bd0319
update
wangzelin007 Dec 13, 2023
0a0f70b
Update azdev_linter_style.py
wangzelin007 Dec 13, 2023
93c7c14
update
wangzelin007 Dec 13, 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
22 changes: 14 additions & 8 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@
max-line-length = 120
max-complexity = 10
ignore =
E501, # line too long, it is covered by pylint
E722, # bare except, bad practice, to be removed in the future
F401, # imported but unused, too many violations, to be removed in the future
F811, # redefinition of unused, to be removed in the future
C901 # code flow is too complex, too many violations, to be removed in the future
W503 # line break before binary operator effect on readability is subjective
W504 # line break after binary operator effect on readability is subjective

# line too long, it is covered by pylint
E501
# bare except, bad practice, to be removed in the future
E722
# imported but unused, too many violations, to be removed in the future
F401
# redefinition of unused, to be removed in the future
F811
# code flow is too complex, too many violations, to be removed in the future
C901
# line break before binary operator effect on readability is subjective
W503
# line break after binary operator effect on readability is subjective
W504
exclude =
*/vendored_sdks
docs
Expand Down
48 changes: 30 additions & 18 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,6 @@ jobs:
source ./env/bin/activate
azdev verify license

- job: StaticAnalysis
displayName: "Static Analysis"
pool:
name: 'pool-ubuntu-2004'
steps:
- task: UsePythonVersion@0
displayName: 'Use Python 3.6'
inputs:
versionSpec: 3.6
- bash: pip install wheel==0.30.0 pylint==1.9.5 flake8==3.5.0 requests
displayName: 'Install wheel, pylint, flake8, requests'
- bash: python scripts/ci/source_code_static_analysis.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scripts/ci/source_code_static_analysis.py can also be removed.

displayName: "Static Analysis"

- job: IndexVerify
displayName: "Verify Extensions Index"
pool:
Expand Down Expand Up @@ -131,9 +117,10 @@ jobs:
ADO_PULL_REQUEST_LATEST_COMMIT: HEAD
ADO_PULL_REQUEST_TARGET_BRANCH: $(System.PullRequest.TargetBranch)

- job: LintModifiedExtensions
displayName: "CLI Linter on Modified Extensions"
- job: AzdevStyleModifiedExtensions
displayName: "azdev style on Modified Extensions"
condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'))
continueOnError: true
pool:
name: 'pool-ubuntu-2004'
steps:
Expand All @@ -149,8 +136,33 @@ jobs:
# overwrite the default AZURE_EXTENSION_DIR set by ADO
AZURE_EXTENSION_DIR=~/.azure/cliextensions az --version

AZURE_EXTENSION_DIR=~/.azure/cliextensions python scripts/ci/verify_linter.py
displayName: "CLI Linter on Modified Extension"
AZURE_EXTENSION_DIR=~/.azure/cliextensions python scripts/ci/azdev_linter_style.py --type style
displayName: "azdev style on Modified Extensions"
env:
ADO_PULL_REQUEST_LATEST_COMMIT: HEAD
ADO_PULL_REQUEST_TARGET_BRANCH: $(System.PullRequest.TargetBranch)

- job: AzdevLinterModifiedExtensions
displayName: "azdev linter on Modified Extensions"
condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'))
pool:
name: 'pool-ubuntu-2004'
steps:
- task: UsePythonVersion@0
displayName: 'Use Python 3.11'
inputs:
versionSpec: 3.11
- template: .azure-pipelines/templates/azdev_setup.yml
- bash: |
#!/usr/bin/env bash
set -ev
source ./env/bin/activate
# overwrite the default AZURE_EXTENSION_DIR set by ADO
AZURE_EXTENSION_DIR=~/.azure/cliextensions az --version

# TODO: remove --type linter once all extensions are fixed
AZURE_EXTENSION_DIR=~/.azure/cliextensions python scripts/ci/azdev_linter_style.py --type linter
displayName: "azdev linter on Modified Extensions"
env:
ADO_PULL_REQUEST_LATEST_COMMIT: HEAD
ADO_PULL_REQUEST_TARGET_BRANCH: $(System.PullRequest.TargetBranch)
Expand Down
75 changes: 45 additions & 30 deletions pylintrc
Original file line number Diff line number Diff line change
@@ -1,27 +1,58 @@
[MASTER]
ignore=tests,generated,vendored_sdks,privates
ignore-patterns=test.*,azure_devops_build.*
ignore-paths=.*/aaz
reports=no

[MESSAGES CONTROL]
# For all codes, run 'pylint --list-msgs' or go to 'https://pylint.readthedocs.io/en/latest/reference_guide/features.html'
# cyclic-import: because of https://github.com/PyCQA/pylint/issues/850
# import-outside-toplevel: Lazy import to improve performance
# locally-disabled: Warning locally suppressed using disable-msg
# cyclic-import: Because of https://github.com/PyCQA/pylint/issues/850
# too-many-arguments: Due to the nature of the CLI many commands have large arguments set which reflect in large arguments set in corresponding methods.
# import-outside-toplevel: Lazy import to improve performance
# consider-using-f-string: str.format is still a Python standard
# unspecified-encoding: Allow using the system encoding, instead of forcing utf-8 to avoid opening errors
disable=
missing-docstring,
locally-disabled,
fixme,
arguments-out-of-order,
bad-option-value,
consider-using-with,
cyclic-import,
too-many-arguments,
invalid-name,
duplicate-code,
fixme,
import-outside-toplevel,
too-many-lines

[TYPECHECK]
# For Azure CLI extensions, we ignore some import errors as they'll be available in the environment of the CLI
ignored-modules=azure,azure.cli,azure.cli.core,azure.cli.core.commands,knack,msrestazure,argcomplete,azure.cli.testsdk,isodate,OpenSSL
inconsistent-return-statements,
invalid-name,
invalid-str-returned,
locally-disabled,
missing-docstring,
missing-kwoa,
no-member,
no-value-for-parameter,
raise-missing-from,
subprocess-run-check,
super-with-arguments,
too-many-arguments,
too-many-function-args,
too-many-lines,
using-constant-test,
wrong-import-order,
consider-using-f-string,
unspecified-encoding,
use-maxsplit-arg,
arguments-renamed,
consider-using-in,
use-dict-literal,
consider-using-dict-items,
consider-using-enumerate,
redundant-u-string-prefix,
raising-bad-type,
unused-private-member,
used-before-assignment,
# These rules were added in Pylint >= 2.12, disable them to avoid making retroactive change
missing-timeout,
superfluous-parens,
implicit-str-concat,
unnecessary-dunder-call,

[FORMAT]
max-line-length=120
Expand All @@ -35,24 +66,8 @@ init-import=yes
max-locals=25
# Maximum number of branch for function / method body
max-branches=20
# Temporally disable this check as inline disable is not working, see https://github.com/pylint-dev/pylint/issues/8806
max-statements=500

[SIMILARITIES]
min-similarity-lines=10

[BASIC]
# Naming hints based on PEP 8 (https://www.python.org/dev/peps/pep-0008/#naming-conventions).
# Consider these guidelines and not hard rules. Read PEP 8 for more details.

# The invalid-name checker must be **enabled** for these hints to be used.
include-naming-hint=yes

module-name-hint=lowercase (keep short; underscores are discouraged)
const-name-hint=UPPER_CASE_WITH_UNDERSCORES
class-name-hint=CapitalizedWords
class-attribute-name-hint=lower_case_with_underscores
attr-name-hint=lower_case_with_underscores
method-name-hint=lower_case_with_underscores
function-name-hint=lower_case_with_underscores
argument-name-hint=lower_case_with_underscores
variable-name-hint=lower_case_with_underscores
inlinevar-name-hint=lower_case_with_underscores (short is OK)
75 changes: 50 additions & 25 deletions scripts/ci/verify_linter.py → scripts/ci/azdev_linter_style.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,27 @@
# --------------------------------------------------------------------------------------------

"""
This script is used to verify linter on extensions.
This script is used to run azdev linter and azdev style on extensions.

It's only working on ADO by default. If want to run locally,
please update the target branch/commit to find diff in function find_modified_files_against_master_branch()
"""

import os
import json
from subprocess import check_output, check_call
from pkg_resources import parse_version
import logging
import os
from subprocess import check_call, check_output

import service_name
from pkg_resources import parse_version

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)
ch = logging.StreamHandler()
ch.setLevel(logging.DEBUG)
logger.addHandler(ch)

def separator_line():
print('-' * 100)
logger.info('-' * 100)


class ModifiedFilesNotAllowedError(Exception):
Expand Down Expand Up @@ -50,7 +55,7 @@ def __init__(self, extension_name):

@staticmethod
def _cmd(cmd):
print(cmd)
logger.info(cmd)
check_call(cmd, shell=True)

def add_from_url(self, url):
Expand All @@ -66,7 +71,7 @@ def __init__(self, extension_name):

@staticmethod
def _cmd(cmd):
print(cmd)
logger.info(cmd)
check_call(cmd, shell=True)

def add_from_code(self):
Expand All @@ -78,6 +83,9 @@ def remove(self):
def linter(self):
self._cmd('azdev linter --include-whl-extensions {}'.format(self.extension_name))

def style(self):
self._cmd('azdev style {}'.format(self.extension_name))

def build(self):
pass

Expand All @@ -91,17 +99,17 @@ def find_modified_files_against_master_branch():
ado_pr_target_branch = 'origin/' + os.environ.get('ADO_PULL_REQUEST_TARGET_BRANCH')

separator_line()
print('pull request target branch:', ado_pr_target_branch)
logger.info('pull request target branch: %s', ado_pr_target_branch)

cmd = 'git --no-pager diff --name-only --diff-filter=ACMRT {} -- src/'.format(ado_pr_target_branch)
files = check_output(cmd.split()).decode('utf-8').split('\n')
files = [f for f in files if len(f) > 0]

if files:
print('modified files:')
logger.info('modified files:')
separator_line()
for f in files:
print(f)
logger.info(f)

return files

Expand Down Expand Up @@ -131,10 +139,10 @@ def contain_extension_code(files):
return False


def linter_on_external_extension(index_json):
def azdev_on_external_extension(index_json, azdev_type):
"""
Check if the modified metadata items in index.json refer to the extension in repo.
If not, az extension linter on wheel. Otherwise skip it.
If not, az extension check on wheel. Otherwise skip it.
"""

public_extensions = json.loads(check_output('az extension list-available -d', shell=True))
Expand All @@ -160,15 +168,20 @@ def linter_on_external_extension(index_json):
az_extension.add_from_url(latest_entry['downloadUrl'])

azdev_extension = AzdevExtensionHelper(name)
azdev_extension.linter()

print('Checking service name for external extensions')
if azdev_type in ['all', 'linter']:
azdev_extension.linter()
# TODO:
# azdev style support external extension
# azdev test support external extension
# azdev_extension.style()

logger.info('Checking service name for external extensions')
service_name.check()

az_extension.remove()


def linter_on_internal_extension(modified_files):
def azdev_on_internal_extension(modified_files, azdev_type):
extension_names = set()

for f in modified_files:
Expand All @@ -178,31 +191,43 @@ def linter_on_internal_extension(modified_files):

if not extension_names:
separator_line()
print('no extension source code modified, no extension needs to be linter')
logger.info('no extension source code modified, no extension needs to be checked')

for name in extension_names:
separator_line()

azdev_extension = AzdevExtensionHelper(name)
azdev_extension.add_from_code()
azdev_extension.linter()
if azdev_type in ['all', 'linter']:
azdev_extension.linter()
if azdev_type in ['all', 'style']:
azdev_extension.style()

print('Checking service name for internal extensions')
logger.info('Checking service name for internal extensions')
service_name.check()

azdev_extension.remove()


def main():
import argparse
parser = argparse.ArgumentParser(description='azdev linter and azdev style on modified extensions')
parser.add_argument('--type',
type=str,
help='Control whether azdev linter, azdev style, azdev test needs to be run. '
'Supported values: linter, style, test, all, all is the default.', default='all')
args = parser.parse_args()
azdev_type = args.type
logger.info('azdev type: %s', azdev_type)
modified_files = find_modified_files_against_master_branch()

if len(modified_files) == 1 and contain_index_json(modified_files):
# Scenario 1.
# This scenarios is for modify index.json only.
# If the modified metadata items refer to the extension code exits in this repo, PR is be created via Pipeline.
# If the modified metadata items refer to the extension code doesn't exist, PR is created from Service Team.
# We try to verify linter on it.
linter_on_external_extension(modified_files[0])
# We try to run azdev linter and azdev style on it.
azdev_on_external_extension(modified_files[0], azdev_type)
else:
# modified files contain more than one file

Expand All @@ -211,16 +236,16 @@ def main():
if contain_index_json(modified_files):
raise ModifiedFilesNotAllowedError()

linter_on_internal_extension(modified_files)
azdev_on_internal_extension(modified_files, azdev_type)
else:
separator_line()
print('no extension source code modified, no extension needs to be linter')
logger.info('no extension source code modified, no extension needs to be checked')
separator_line()


if __name__ == '__main__':
try:
main()
except ModifiedFilesNotAllowedError as e:
print(e)
logger.error(e)
exit(1)
Loading