-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Verify autorest in pipeline #16667
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
Verify autorest in pipeline #16667
Changes from 35 commits
fbc3261
03dcdf4
31b4333
7fee4f7
791b909
d1d8cf9
bbffe46
8a6a96d
8f9f913
1b81463
25dea73
60e3869
98f8f5c
52d9b22
9519a2d
5592a03
1a2fd97
019d2d8
3be480a
9c99288
422591b
1a41e66
ba004bc
9214c10
04ab76c
1f67ec5
158457d
68b818e
f7a200c
51c07ed
b546a87
83168da
13f7327
f39d4af
7046828
b79fe26
235c363
26ca1e8
fc6c93d
b3b7e06
fd3dee0
031c32c
7bf5442
af59dd2
4c63c1c
7f79740
0226f0e
2ec2b9e
efd3be9
6c49c4c
df8676a
eeb61e7
075f040
0b23815
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| parameters: | ||
| - name: ServiceToTest | ||
| type: string | ||
| default: '' | ||
| - name: SDKType | ||
| type: string | ||
| default: 'client' | ||
| - name: ServiceDirectory | ||
| type: string | ||
| - name: NodeVersion | ||
| type: string | ||
| default: '12.x' | ||
| - name: PythonVersion | ||
| type: string | ||
| default: '$(PythonVersion)' | ||
| - name: auto_rest_clone_url | ||
| type: string | ||
| default: 'https://github.com/Azure/autorest.python.git' | ||
|
|
||
| steps: | ||
| - task: UsePythonVersion@0 | ||
| displayName: Use | ||
| inputs: | ||
| versionSpec: ${{ parameters.PythonVersion }} | ||
|
|
||
| - task: NodeTool@0 | ||
| inputs: | ||
| versionSpec: ${{ parameters.NodeVersion }} | ||
|
|
||
| - script: | | ||
| npm install -g autorest | ||
| autorest --help | ||
| displayName: "Install autorest" | ||
|
|
||
| - script: | | ||
| python --version | ||
| pip install -r eng/autorest_req.txt | ||
| git clone ${{ parameters.auto_rest_clone_url }} | ||
| cd autorest.python | ||
| npm install | ||
| displayName: 'Prepare Environment' | ||
|
|
||
| - task: PythonScript@0 | ||
| displayName: 'Run autorest Python' | ||
| inputs: | ||
| scriptPath: 'scripts/devops_tasks/verify_autorest.py' | ||
| arguments: >- | ||
| --service_directory="${{ parameters.ServiceDirectory }}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| #!/usr/bin/env python | ||
|
|
||
| # -------------------------------------------------------------------------------------------- | ||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||
| # Licensed under the MIT License. See License.txt in the project root for license information. | ||
| # -------------------------------------------------------------------------------------------- | ||
|
|
||
| import argparse | ||
| from git import Repo | ||
| import os | ||
| import logging | ||
| import sys | ||
|
|
||
| from common_tasks import run_check_call | ||
|
|
||
| logging.getLogger().setLevel(logging.INFO) | ||
|
|
||
| root_dir = os.path.abspath(os.path.join(os.path.abspath(__file__), "..", "..", "..")) | ||
| sdk_dir = os.path.join(root_dir, "sdk") | ||
|
|
||
| SWAGGER_FOLDER = "swagger" | ||
|
|
||
|
|
||
| def run_autorest(service_dir): | ||
| logging.info("Running autorest for {}".format(service_dir)) | ||
|
|
||
| service_dir = os.path.join(sdk_dir, service_dir) | ||
|
|
||
| swagger_folders = find_swagger_folders(service_dir) | ||
|
|
||
| for working_dir in swagger_folders: | ||
| os.chdir(working_dir) | ||
| f = os.path.join(working_dir, "README.md") | ||
| if os.path.exists(f): | ||
| reset_command = ["autorest", "--reset"] | ||
| run_check_call(reset_command, root_dir) | ||
|
|
||
| command = ["autorest", "--python", f, "--verbose"] | ||
| logging.info("Command: {}\nLocation: {}\n".format(command, working_dir)) | ||
| run_check_call(command, working_dir) | ||
| return swagger_folders | ||
|
|
||
|
|
||
| def find_swagger_folders(directory): | ||
| logging.info("Searching for swagger files in: {}".format(directory)) | ||
|
|
||
| ret = [] | ||
| for root, subdirs, files in os.walk(directory): | ||
| for d in subdirs: | ||
| if d == SWAGGER_FOLDER: | ||
| if os.path.exists(os.path.join(root, d, "README.md")): | ||
| ret.append(os.path.join(root, d)) | ||
|
|
||
| logging.info("Found swagger files at: {}".format(ret)) | ||
| return ret | ||
|
|
||
|
|
||
| def check_diff(): | ||
| repo = Repo(root_dir) | ||
| t = repo.head.commit.tree | ||
| d = repo.git.diff(t) | ||
| if d: | ||
| command = ["git", "status"] | ||
| run_check_call(command, root_dir) | ||
| raise ValueError( | ||
| "Found difference between re-generated code and current commit. Please re-generate with the latest autorest." | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| parser = argparse.ArgumentParser( | ||
| description="Run autorest to verify generated code." | ||
| ) | ||
| parser.add_argument( | ||
| "--service_directory", help="Directory of the package being tested" | ||
| ) | ||
|
|
||
| args = parser.parse_args() | ||
| folders = run_autorest(args.service_directory) | ||
|
|
||
| if len(folders): | ||
| check_diff() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,4 +40,5 @@ extends: | |
| - name: azure-mgmt-containerregistry | ||
| safeName: azuremgmtcontainerregistry | ||
| - name: azure-containerregistry | ||
| safeName: azurecontainerregistry | ||
| safeName: azurecontainerregistry | ||
| VerifyAutorest: false | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In these the udpated ci.yml, Trying to get a feel for the impact if we check this in. Also, have you let the mgmt folks know about this new check?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this will cause issues with containerregistry and storage from testing it on both. I will reach out to the storage team to see if they want me to default it on. I have not let mgmt team know about it, but will reach out. |
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, as I don't believe autorest repo is that big. but can we use a sparse checkout?
I'm like 99% certain this will work. This will basically grab zero history for the commitsh you ask for. I'm adding path "/" to tell it to grab every file in the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm testing this locally to confirm that it's good advice. Feel free to ignore this comment for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I tested this locally.
This will not work for what I'm asking you to do until I update the
sparse-checkout.ymlto NOT grab the folderengby default. It would work if the pattern was simply:But because by default it has a
Run before it adds the path, it actually won't work for git repos without an
engfolder. It just quietly doesn't checkout properly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR that will enable this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@scbedd I would set the path to
/*instead of/which will work witheng. This is what git sparse-checkout defaults to using anyway before we rungit sparse-checkout setThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. I'll definitely give it a shot locally.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seankane-msft this works. I updated the example above. I believe it should be a drop in replacement.