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

Update Python SDK to Track 2 #1649

Merged
merged 2 commits into from Aug 18, 2021
Merged

Conversation

25region
Copy link
Contributor

@25region 25region commented Jul 30, 2021

Which issue this PR addresses:

Migrate ARO cli extension to SDK v2

What this PR does / why we need it:

The aro CLI extension currently uses v1 of Azure SDK for Python, which will be decommissioned in early 2022.

The goal of this story is to convert the aro cli extension to work with Azure SDK v2. There will be breaking changes after bumping the SDK to v2 and we need to guarantee all the existing functionalities are working with the new SDK version.

Test plan for issue:

  • Tested az aro commands with the new Track 2 extension
  • Made changes submitted to solve issues with failed commands

Is there any documentation that needs to be updated for this PR?

Follow up stories will need to be created:

  • Push Azure-cli changes upstream
  • Migrate Go SDK to Track 2

hack/build-client.sh Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@@ -0,0 +1,23 @@
FROM registry.access.redhat.com/ubi8/nodejs-14
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not be LTS after 2021-10-26 which will be nodejs-16. Nodejs 14 will be in maintenance but maybe a comment to change this image or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, AutoRest warns about versions higher than 14. We can update the base image once this warning goes away:
WARNING: AutoRest has not been tested with Node versions greater than v14.

Makefile Show resolved Hide resolved
hack/build-client.sh Outdated Show resolved Hide resolved
@troy0820 troy0820 added priority-medium Medium priority issue or pull request size-medium Size medium labels Aug 3, 2021
@mjudeikis
Copy link
Contributor

@25region please commit other changes together here. Im not gonna merge this without the changes. This PR now is bit out of context as it looks like it is contains all the changes, but it does not.
And the fact our CI is not picking it too is an issue.. But separate one .

@github-actions github-actions bot added the needs-rebase branch needs a rebase label Aug 5, 2021
@github-actions
Copy link

github-actions bot commented Aug 5, 2021

Please rebase pull request.

@25region
Copy link
Contributor Author

25region commented Aug 5, 2021

@25region please commit other changes together here. Im not gonna merge this without the changes. This PR now is bit out of context as it looks like it is contains all the changes, but it does not.
And the fact our CI is not picking it too is an issue.. But separate one .

@mjudeikis
Committed SDK generated changes, vendor modules, and python client changes.

@25region 25region closed this Aug 10, 2021
@25region 25region reopened this Aug 11, 2021
@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Aug 11, 2021
@25region 25region force-pushed the autorest-image branch 4 times, most recently from a5be3e2 to 22d7110 Compare August 11, 2021 16:09
@25region
Copy link
Contributor Author

25region commented Aug 11, 2021

@mjudeikis - reverted to switch only Python SDK to Track 2.
az aro create is getting the following error:

(InvalidParameter) The provided worker VM size 'Standard_D2s_v3' is invalid.

Copy link
Contributor

@mjudeikis mjudeikis left a comment

Choose a reason for hiding this comment

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

@mjudeikis - reverted to switch only Python SDK to Track 2.
az aro create is getting the following error:

(InvalidParameter) The provided worker VM size 'Standard_D2s_v3' is invalid.

Gonna leave this here :) Are you using development RP?

if requiredD2sV3Workers {

Overall looks good just tests and nits

@25region 25region changed the title Update AutoRest to Track 2 Update Python SDK to Track 2 Aug 13, 2021
@25region 25region marked this pull request as ready for review August 17, 2021 21:16
@25region 25region requested a review from jewzaam as a code owner August 17, 2021 21:16
@mjudeikis
Copy link
Contributor

Thanks for this! Next step is to upstream this to azure/azure-cli repo! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-medium Medium priority issue or pull request size-medium Size medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants