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

Adding required attributes to location and properties #9777

Merged
merged 2 commits into from
Jun 11, 2020

Conversation

bganapa
Copy link
Member

@bganapa bganapa commented Jun 8, 2020

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

Contribution checklist:

If any further question about AME onboarding or validation tools, please view the FAQ.

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.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jun 8, 2020

[Staging] Swagger Validation Report

~[Staging] BreakingChange: 2 Errors, 0 Warnings [Detail][Expand Details]
Rule Message
1034 - AddedRequiredProperty The new version has new required property 'properties, location' that was not found in the old version.
New: Microsoft.AzureStack/stable/2017-06-01/Registration.json#L152:9
1034 - AddedRequiredProperty The new version has new required property 'properties, location' that was not found in the old version.
New: Microsoft.AzureStack/stable/2017-06-01/Registration.json#L327:5
Old: Microsoft.AzureStack/stable/2017-06-01/Registration.json#L327:5
Posted by Swagger Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 8, 2020

azure-sdk-for-go - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 8, 2020

azure-sdk-for-net - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 8, 2020

azure-sdk-for-js - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from 62b1f2b with merge commit 0e03636. SDK Automation 13.0.17.20200605.3
  • ️✔️@azure/arm-azurestack [View full logs]  [Release SDK Changes]
    Only show 100 items here, please refer to log for details.
    [npmPack] npm WARN deprecated [email protected]: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-node-resolve.
    [npmPack] npm WARN deprecated [email protected]: https://github.com/lydell/resolve-url#deprecated
    [npmPack] npm WARN deprecated [email protected]: Please see https://github.com/lydell/urix#deprecated
    [npmPack] npm notice created a lockfile as package-lock.json. You should commit this file.
    [npmPack] loaded rollup.config.js with warnings
    [npmPack] (!) Unused external imports
    [npmPack] default imported from external module 'rollup' but never used
    [npmPack] 
    [npmPack] ./esm/azureStackManagementClient.js → ./dist/arm-azurestack.js...
    [npmPack] created ./dist/arm-azurestack.js in 317ms
    [npmPack] npm notice 
    [npmPack] npm notice package: @azure/[email protected]
    [npmPack] npm notice === Tarball Contents === 
    [npmPack] npm notice 79.7kB  dist/arm-azurestack.js                          
    [npmPack] npm notice 28.3kB  dist/arm-azurestack.min.js                      
    [npmPack] npm notice 2.0kB   esm/azureStackManagementClient.js               
    [npmPack] npm notice 2.7kB   esm/azureStackManagementClientContext.js        
    [npmPack] npm notice 2.5kB   esm/operations/cloudManifestFile.js             
    [npmPack] npm notice 661B    esm/models/cloudManifestFileMappers.js          
    [npmPack] npm notice 6.5kB   esm/operations/customerSubscriptions.js         
    [npmPack] npm notice 703B    esm/models/customerSubscriptionsMappers.js      
    [npmPack] npm notice 345B    esm/models/index.js                             
    [npmPack] npm notice 520B    esm/operations/index.js                         
    [npmPack] npm notice 29.8kB  esm/models/mappers.js                           
    [npmPack] npm notice 2.2kB   esm/operations/operations.js                    
    [npmPack] npm notice 440B    esm/models/operationsMappers.js                 
    [npmPack] npm notice 2.6kB   esm/models/parameters.js                        
    [npmPack] npm notice 8.4kB   esm/operations/products.js                      
    [npmPack] npm notice 784B    esm/models/productsMappers.js                   
    [npmPack] npm notice 7.9kB   esm/operations/registrations.js                 
    [npmPack] npm notice 731B    esm/models/registrationsMappers.js              
    [npmPack] npm notice 992B    rollup.config.js                                
    [npmPack] npm notice 1.7kB   package.json                                    
    [npmPack] npm notice 457B    tsconfig.json                                   
    [npmPack] npm notice 171.0kB dist/arm-azurestack.js.map                      
    [npmPack] npm notice 24.4kB  dist/arm-azurestack.min.js.map                  
    [npmPack] npm notice 779B    esm/azureStackManagementClient.d.ts.map         
    [npmPack] npm notice 963B    esm/azureStackManagementClient.js.map           
    [npmPack] npm notice 519B    esm/azureStackManagementClientContext.d.ts.map  
    [npmPack] npm notice 1.4kB   esm/azureStackManagementClientContext.js.map    
    [npmPack] npm notice 1.1kB   esm/operations/cloudManifestFile.d.ts.map       
    [npmPack] npm notice 1.5kB   esm/operations/cloudManifestFile.js.map         
    [npmPack] npm notice 349B    esm/models/cloudManifestFileMappers.d.ts.map    
    [npmPack] npm notice 362B    esm/models/cloudManifestFileMappers.js.map      
    [npmPack] npm notice 2.8kB   esm/operations/customerSubscriptions.d.ts.map   
    [npmPack] npm notice 3.6kB   esm/operations/customerSubscriptions.js.map     
    [npmPack] npm notice 380B    esm/models/customerSubscriptionsMappers.d.ts.map
    [npmPack] npm notice 393B    esm/models/customerSubscriptionsMappers.js.map  
    [npmPack] npm notice 11.4kB  esm/models/index.d.ts.map                       
    [npmPack] npm notice 227B    esm/operations/index.d.ts.map                   
    [npmPack] npm notice 126B    esm/models/index.js.map                         
    [npmPack] npm notice 242B    esm/operations/index.js.map                     
    [npmPack] npm notice 1.7kB   esm/models/mappers.d.ts.map                     
    [npmPack] npm notice 17.0kB  esm/models/mappers.js.map                       
    [npmPack] npm notice 1.0kB   esm/operations/operations.d.ts.map              
    [npmPack] npm notice 1.4kB   esm/operations/operations.js.map                
    [npmPack] npm notice 209B    esm/models/operationsMappers.d.ts.map           
    [npmPack] npm notice 222B    esm/models/operationsMappers.js.map             
    [npmPack] npm notice 639B    esm/models/parameters.d.ts.map                  
    [npmPack] npm notice 1.9kB   esm/models/parameters.js.map                    
    [npmPack] npm notice 3.6kB   esm/operations/products.d.ts.map                
    [npmPack] npm notice 5.0kB   esm/operations/products.js.map                  
    [npmPack] npm notice 417B    esm/models/productsMappers.d.ts.map             
    [npmPack] npm notice 430B    esm/models/productsMappers.js.map               
    [npmPack] npm notice 3.5kB   esm/operations/registrations.d.ts.map           
    [npmPack] npm notice 4.7kB   esm/operations/registrations.js.map             
    [npmPack] npm notice 390B    esm/models/registrationsMappers.d.ts.map        
    [npmPack] npm notice 403B    esm/models/registrationsMappers.js.map          
    [npmPack] npm notice 3.2kB   README.md                                       
    [npmPack] npm notice 1.4kB   esm/azureStackManagementClient.d.ts             
    [npmPack] npm notice 2.0kB   src/azureStackManagementClient.ts               
    [npmPack] npm notice 938B    esm/azureStackManagementClientContext.d.ts      
    [npmPack] npm notice 2.4kB   src/azureStackManagementClientContext.ts        
    [npmPack] npm notice 2.1kB   esm/operations/cloudManifestFile.d.ts           
    [npmPack] npm notice 4.4kB   src/operations/cloudManifestFile.ts             
    [npmPack] npm notice 351B    esm/models/cloudManifestFileMappers.d.ts        
    [npmPack] npm notice 640B    src/models/cloudManifestFileMappers.ts          
    [npmPack] npm notice 7.3kB   esm/operations/customerSubscriptions.d.ts       
    [npmPack] npm notice 13.5kB  src/operations/customerSubscriptions.ts         
    [npmPack] npm notice 393B    esm/models/customerSubscriptionsMappers.d.ts    
    [npmPack] npm notice 682B    src/models/customerSubscriptionsMappers.ts      
    [npmPack] npm notice 30.0kB  esm/models/index.d.ts                           
    [npmPack] npm notice 204B    esm/operations/index.d.ts                       
    [npmPack] npm notice 28.7kB  src/models/index.ts                             
    [npmPack] npm notice 488B    src/operations/index.ts                         
    [npmPack] npm notice 2.1kB   esm/models/mappers.d.ts                         
    [npmPack] npm notice 23.7kB  src/models/mappers.ts                           
    [npmPack] npm notice 2.1kB   esm/operations/operations.d.ts                  
    [npmPack] npm notice 4.1kB   src/operations/operations.ts                    
    [npmPack] npm notice 130B    esm/models/operationsMappers.d.ts               
    [npmPack] npm notice 404B    src/models/operationsMappers.ts                 
    [npmPack] npm notice 768B    esm/models/parameters.d.ts                      
    [npmPack] npm notice 2.6kB   src/models/parameters.ts                        
    [npmPack] npm notice 8.9kB   esm/operations/products.d.ts                    
    [npmPack] npm notice 17.1kB  src/operations/products.ts                      
    [npmPack] npm notice 474B    esm/models/productsMappers.d.ts                 
    [npmPack] npm notice 788B    src/models/productsMappers.ts                   
    [npmPack] npm notice 8.4kB   esm/operations/registrations.d.ts               
    [npmPack] npm notice 16.2kB  src/operations/registrations.ts                 
    [npmPack] npm notice 421B    esm/models/registrationsMappers.d.ts            

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 8, 2020

Azure CLI Extension Generation - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 8, 2020

azure-sdk-for-python - Release

- Breaking Change detected in SDK

⚠️ warning [Logs] [Expand Details]
  • ⚠️ Generate from 62b1f2b with merge commit 0e03636. SDK Automation 13.0.17.20200605.3
  • ⚠️azure-mgmt-azurestack [View full logs]  [Release SDK Changes] Breaking Change Detected
    [build_conf] INFO:packaging_tools:Building template azure-mgmt-azurestack
    [build_conf] INFO:packaging_tools.conf:Skipping default conf since the file exists
    [build_conf] INFO:packaging_tools:Skipping CHANGELOG.md template, since a previous one was found
    [build_conf] INFO:packaging_tools:Template done azure-mgmt-azurestack
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
    [build_package]   warnings.warn(msg)
    [build_package] warning: no files found matching '*.py' under directory 'tests'
    [build_package] warning: no files found matching '*.yaml' under directory 'tests'
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
    [build_package]   warnings.warn(msg)
    [build_package] warning: no files found matching '*.py' under directory 'tests'
    [build_package] warning: no files found matching '*.yaml' under directory 'tests'
    [breaking_change_setup] Ignoring mock: markers 'python_version <= "2.7"' don't match your environment
    [ChangeLog] Size of delta 10.340% size of original (original: 21943 chars, delta: 2269 chars)
    [ChangeLog] **Features**
    [ChangeLog] 
    [ChangeLog]   - Added operation group CloudManifestFileOperations
    [ChangeLog] 
    [ChangeLog] **Breaking changes**
    [ChangeLog] 
    [ChangeLog]   - Parameter location of model RegistrationParameter is now required
    [ChangeLog]   - Operation RegistrationsOperations.update has a new signature
    [ChangeLog]   - Operation RegistrationsOperations.create_or_update has a new signature

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 8, 2020

azure-sdk-for-java - Release

⚠️ warning [Logs] [Expand Details]
  • ⚠️ Generate from 62b1f2b with merge commit 0e03636. SDK Automation 13.0.17.20200605.3
  • ⚠️sdk/azurestack/mgmt-v2017_06_01 [View full logs]  [Release SDK Changes]
      [mvn] [ERROR] COMPILATION ERROR : 
      [mvn] [ERROR] /z/work/azure-sdk-for-java/sdk/azurestack/mgmt-v2017_06_01/src/main/java/com/microsoft/azure/management/azurestack/v2017_06_01/implementation/ProductsInner.java:[469,155] cannot find symbol
      [mvn]   symbol:   variable deviceConfiguration
      [mvn]   location: class com.microsoft.azure.management.azurestack.v2017_06_01.implementation.ProductsInner
      [mvn] [ERROR] /z/work/azure-sdk-for-java/sdk/azurestack/mgmt-v2017_06_01/src/main/java/com/microsoft/azure/management/azurestack/v2017_06_01/implementation/ProductsInner.java:[562,167] cannot find symbol
      [mvn]   symbol:   variable deviceConfiguration
      [mvn]   location: class com.microsoft.azure.management.azurestack.v2017_06_01.implementation.ProductsInner
      [mvn] [ERROR] /z/work/azure-sdk-for-java/sdk/azurestack/mgmt-v2017_06_01/src/main/java/com/microsoft/azure/management/azurestack/v2017_06_01/implementation/ProductsInner.java:[655,166] cannot find symbol
      [mvn]   symbol:   variable marketplaceProductLogUpdate
      [mvn]   location: class com.microsoft.azure.management.azurestack.v2017_06_01.implementation.ProductsInner
      [mvn] [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project azure-mgmt-azurestack: Compilation failure: Compilation failure: 
      [mvn] [ERROR] /z/work/azure-sdk-for-java/sdk/azurestack/mgmt-v2017_06_01/src/main/java/com/microsoft/azure/management/azurestack/v2017_06_01/implementation/ProductsInner.java:[469,155] cannot find symbol
      [mvn] [ERROR]   symbol:   variable deviceConfiguration
      [mvn] [ERROR]   location: class com.microsoft.azure.management.azurestack.v2017_06_01.implementation.ProductsInner
      [mvn] [ERROR] /z/work/azure-sdk-for-java/sdk/azurestack/mgmt-v2017_06_01/src/main/java/com/microsoft/azure/management/azurestack/v2017_06_01/implementation/ProductsInner.java:[562,167] cannot find symbol
      [mvn] [ERROR]   symbol:   variable deviceConfiguration
      [mvn] [ERROR]   location: class com.microsoft.azure.management.azurestack.v2017_06_01.implementation.ProductsInner
      [mvn] [ERROR] /z/work/azure-sdk-for-java/sdk/azurestack/mgmt-v2017_06_01/src/main/java/com/microsoft/azure/management/azurestack/v2017_06_01/implementation/ProductsInner.java:[655,166] cannot find symbol
      [mvn] [ERROR]   symbol:   variable marketplaceProductLogUpdate
      [mvn] [ERROR]   location: class com.microsoft.azure.management.azurestack.v2017_06_01.implementation.ProductsInner
      [mvn] [ERROR] -> [Help 1]
      [mvn] [ERROR] 
      [mvn] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
      [mvn] [ERROR] Re-run Maven using the -X switch to enable full debug logging.
      [mvn] [ERROR] 
      [mvn] [ERROR] For more information about the errors and possible solutions, please read the following articles:
      [mvn] [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

    @openapi-sdkautomation
    Copy link

    openapi-sdkautomation bot commented Jun 8, 2020

    Trenton Generation - Release

    No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

    @openapi-sdkautomation
    Copy link

    openapi-sdkautomation bot commented Jun 8, 2020

    azure-sdk-for-python-track2 - Release

    No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

    @azuresdkci
    Copy link
    Contributor

    Can one of the admins verify this patch?

    @raych1
    Copy link
    Member

    raych1 commented Jun 9, 2020

    Hi @bganapa , changing properties as required properties in the stable version is breaking change. Can you change it with a new api version?

    @bganapa
    Copy link
    Member Author

    bganapa commented Jun 9, 2020

    @raych1 This is a bug fix as in #9400 , we are not changing the API

    @bganapa
    Copy link
    Member Author

    bganapa commented Jun 9, 2020

    @raych1 Could you please merge this? I think this issue is showing up in some review as the attached issue is old and longstanding

    @raych1
    Copy link
    Member

    raych1 commented Jun 10, 2020

    Hi @bganapa , please fix the PrettierCheck failures.

    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @bganapa
    Copy link
    Member Author

    bganapa commented Jun 10, 2020

    @raych1 Prettier issues fixed. Could you please merge this. This is high pri. Thanks for prioritizing this!

    @raych1
    Copy link
    Member

    raych1 commented Jun 11, 2020

    Thanks @bganapa , it still needs ARM sign off.

    @bganapa
    Copy link
    Member Author

    bganapa commented Jun 11, 2020

    @raych1 Thanks for the response. Whom should I ping for the ARM signoff? As you can see, there is no service side change, hence it is not a breaking change. We are simply fixing a bug found by the customer. If you can, I would urge you to go ahead and merge this PR as this came up in few review. Thanks for your understanding!

    @majastrz majastrz removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jun 11, 2020
    @majastrz majastrz added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Jun 11, 2020
    Copy link
    Member

    @majastrz majastrz left a comment

    Choose a reason for hiding this comment

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

    Signed off from ARM side.

    @raych1 raych1 added the Approved-OkToMerge <valid label in PR review process>add this label when assignee approve to merge the updates label Jun 11, 2020
    @raych1
    Copy link
    Member

    raych1 commented Jun 11, 2020

    @akning-ms , can you please force merge this PR since it's a bug fix in swagger and no impact in service side?

    @akning-ms akning-ms merged commit 0e03636 into Azure:master Jun 11, 2020
    00Kai0 pushed a commit to 00Kai0/azure-rest-api-specs that referenced this pull request Oct 12, 2020
    * Adding required attributes to location and properties
    
    * Prettier fixes
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Approved-OkToMerge <valid label in PR review process>add this label when assignee approve to merge the updates ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    6 participants