-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Updating the Media Services SDK to support the 2018-06-01-preview ver… #4479
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
Conversation
| return new Tuple<string, string, string>[] | ||
| { | ||
| new Tuple<string, string, string>("Media", "Assets", "2018-06-01-preview"), | ||
| new Tuple<string, string, string>("Media", "ContentKeyPolicies", "2018-06-01-preview"), |
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.
Please run msbuild build.proj /t:build /p:Scope=SDKs\Media and commit changes to the .props file
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 ran "msbuild build.proj /t:build /p:Scope=SDKs\Media" and the build succeeds but it didn't produce any changes to the .props file. I also did a "git clean -xdf", then "msbuild build.proj", and then "msbuild build.proj /t:build /p:Scope=SDKs\Media" but still no update to the props file. Any suggestions?
|
|
||
| [assembly: AssemblyVersion("1.0.0")] | ||
| [assembly: AssemblyFileVersion("1.0.0")] | ||
| [assembly: AssemblyVersion("1.0.1")] |
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.
AssemblyVersion must remain the same, AssemblyFileVersion must be updated
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.
Thanks, fixed.
| Assert.Equal(expectedAssetName, locator.AssetName); | ||
| Assert.Equal(expectedName, locator.Name); | ||
| Assert.Empty(locator.ContentKeys); | ||
| //Assert.NotEmpty(locator.ContentKeys); // TODO: This is currently not implemented consistently. Verify it once it is |
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: Please delete this line instead of commenting
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.
Removed.
| new Tuple<string, string, string>("Media", "Assets", "2018-06-01-preview"), | ||
| new Tuple<string, string, string>("Media", "ContentKeyPolicies", "2018-06-01-preview"), | ||
| new Tuple<string, string, string>("Media", "Jobs", "2018-06-01-preview"), | ||
| new Tuple<string, string, string>("Media", "LiveEvents", "2018-06-01-preview"), |
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.
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.
Done.
| @@ -1,80 +0,0 @@ | |||
| // <auto-generated> | |||
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.
@quintinb Apologize for missing this. Deleting a model type could be a breaking change to users. Please bump the version to 2.0.0 in the csproj
I also see that the API version is preview but the SDKs generated are stable, this is usually not recommended.
@shahabhijeet Please confirm
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 bumped the version number as requested. Generating the SDKs as stable was not intentional. Can you advise on how to generate them as preview next time? Is it by adding preview to the version string instead of just numbers? Thanks.
…ing the AzSdk.RP.props
| <PackageId>Microsoft.Azure.Management.Media</PackageId> | ||
| <Description>Provides developers with libraries for managing Azure Media Services using the Azure Resource Manager API.</Description> | ||
| <VersionPrefix>1.0.0</VersionPrefix> | ||
| <VersionPrefix>1.0.1</VersionPrefix> |
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.
@quintinb any reason you are creating a stable package out of a preview API version?
API version change will have to be reflected on the SDK version with a major version bump.
| StreamingEndpoints = new StreamingEndpointsOperations(this); | ||
| BaseUri = new System.Uri("https://management.azure.com"); | ||
| ApiVersion = "2018-03-30-preview"; | ||
| ApiVersion = "2018-06-01-preview"; |
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.
@quintinb as the API version is changing, you will need to bump up major version for the SDK.
Also the comments below about creating stable package out of preview spec is something that is odd and would like to get clarification on that one.
…s a preview Azure Resource Manager API version
shahabhijeet
left a comment
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.
This is good to go, once the CI passes
This change updates the Media Services SDK to support the 2nd preview version (2018-06-01-preview).
Changes from the 2018-03-30-preview version include:
This is based off the swagger spec in pull request Azure/azure-rest-api-specs#3177 which is approved and waiting to be merged.
Tests have been updated with the new API versions. API version updated in the csproj and assemblyinfo.cs.