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

mgmt, upgrade compute (Gallery) #31322

Merged
merged 18 commits into from
Nov 17, 2022

Conversation

weidongxu-microsoft
Copy link
Member

@weidongxu-microsoft weidongxu-microsoft commented Oct 8, 2022

Live test passes.

There are some minor breaking changes, but user on convenience API likely won't be affected.

Description

Please add an informative description that covers that changes made by the pull request and link all relevant issues.

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@weidongxu-microsoft weidongxu-microsoft added the Mgmt This issue is related to a management-plane library. label Oct 8, 2022
@weidongxu-microsoft weidongxu-microsoft self-assigned this Oct 8, 2022
@azure-sdk
Copy link
Collaborator

azure-sdk commented Oct 8, 2022

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-resourcemanager-compute

@weidongxu-microsoft
Copy link
Member Author

hold on, waiting for explanations from service https://github.com/Azure/sdk-release-request/issues/3235 on API change and potential behavior change.

Comment on lines 9 to 10
- Supported `copyCompletionPercent` and `copyCompletionError` method in `Snapshot` for retrieving `CopyStart` progress.
- Supported `copyCompletionPercent` and `copyCompletionError` method in `Snapshot` for retrieving `CopyStart` progress.
Copy link
Contributor

Choose a reason for hiding this comment

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

These seem to be duplicate.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks. caused by conflict resolve. solved.

- Supported `withTrustedLaunch`, `withSecureBoot` and `withVTpm` methods in `VirtualMachine`.
- Supported `UBUNTU_SERVER_18_04_LTS_GEN2`, `UBUNTU_SERVER_20_04_LTS` and `UBUNTU_SERVER_20_04_LTS_GEN2` in `KnownLinuxVirtualMachineImage`.

### Breaking Changes

- Property `uri` removed from `GalleryArtifactVersionSource`. This property was non-functional.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, this may affect customer calling serviceClient here:
#31860 (comment)

though I doubt their code ever run. I will provide them with an example with convenience code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it. Thought about whether to rename the new GalleryDiskImageSource to previous GalleryArtifactVersionSource (as it got uri but one more property).
And it might get less breaks.

However, this could make the model definition in Java too diverged from what intended by service.

@weidongxu-microsoft weidongxu-microsoft merged commit 1c6b5e7 into Azure:main Nov 17, 2022
@weidongxu-microsoft weidongxu-microsoft deleted the mgmt_compute branch November 17, 2022 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants