Skip to content

Add filter to StreamingLocator and Streaming presets.#5340

Merged
anuchandy merged 1 commit intoAzure:masterfrom
BrianBlum:dev-mediaservices-Microsoft.Media-2018-07-01
Mar 15, 2019
Merged

Add filter to StreamingLocator and Streaming presets.#5340
anuchandy merged 1 commit intoAzure:masterfrom
BrianBlum:dev-mediaservices-Microsoft.Media-2018-07-01

Conversation

@BrianBlum
Copy link

@BrianBlum BrianBlum commented Mar 7, 2019

Documentation fixes.

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

@AutorestCI
Copy link

AutorestCI commented Mar 7, 2019

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Mar 7, 2019

Automation for azure-sdk-for-ruby

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-ruby#2327

@AutorestCI
Copy link

AutorestCI commented Mar 7, 2019

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#4289

@AutorestCI
Copy link

AutorestCI commented Mar 7, 2019

Automation for azure-sdk-for-node

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-node#4924

@AutorestCI
Copy link

AutorestCI commented Mar 7, 2019

Automation for azure-sdk-for-js

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-js#1585

@AutorestCI
Copy link

AutorestCI commented Mar 7, 2019

Automation for azure-sdk-for-java

Encountered a Subprocess error: (azure-sdk-for-java)

Command: ['/usr/local/bin/autorest', '/tmp/tmpu8rejgh0/rest/specification/mediaservices/resource-manager/readme.md', '--azure-libraries-for-java-folder=/tmp/tmpu8rejgh0/sdk', '--java', '--multiapi', '--use=@microsoft.azure/autorest.java@2.1.85', '--verbose']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4283; node: v8.12.0]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
Failure:
Error: Unable to acquire shared lock on '/root/.autorest' before timeout 20000 msec.
Error: Unable to acquire shared lock on '/root/.autorest' before timeout 20000 msec.
    at SharedLock.acquire (/node_modules/@microsoft.azure/tasks/dist/lib/exclusive-locks.js:254:19)
    at <anonymous>

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 7, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5340'
REST Spec PR Author 'BrianBlum'
REST Spec PR Last commit
Copy link
Member

@anuchandy anuchandy left a comment

Choose a reason for hiding this comment

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

This PR mark few existing properties are required. Are these properties required since the introduction of this API Version or is this a breaking change?

"codecs",
"formats"
],
"description": "Describes all the settings to be used when encoding the input video with the Standard Encoder."
Copy link
Member

Choose a reason for hiding this comment

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

Changing properties from optional to required is a breaking change. Are these properties required since the introduction of this API Version?

Copy link
Author

@BrianBlum BrianBlum Mar 11, 2019

Choose a reason for hiding this comment

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

These properties have been required since the introduction of this version of the API. The API calls were failing if these properties weren't provided. This change just moves the validation to the client.

},
"type": "object",
"required": [
"labels"
Copy link
Member

Choose a reason for hiding this comment

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

Changing properties from optional to required is a breaking change. Are these properties required since the introduction of this API Version?

Copy link
Author

Choose a reason for hiding this comment

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

The OutputFile object which has the labels is an optional property, which no one specifies (based on Kusto queries across all DCs). If someone were to specify an outputFile, they would want to specify the label, which acts as a directive to media processor, telling it how to combine bitrates.
We don't consider this to be a breaking change.

@BrianBlum
Copy link
Author

BrianBlum commented Mar 13, 2019 via email

Copy link
Member

@anuchandy anuchandy left a comment

Choose a reason for hiding this comment

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

Checked with Media team - these are not breaking change in the service side. PR is correcting swagger to reflect the contract.

Response from team:

 The label is required on the backend and the server would throw an error if label is not set.  
 The swagger change is just make the check early on the client side and make it consistent 
 with the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments