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

Add 201 response to AppServicePlans_CreateOrUpdate #4351

Merged
merged 2 commits into from
Feb 26, 2019

Conversation

pietro
Copy link
Contributor

@pietro pietro commented Oct 29, 2018

Fixes #3648.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Oct 29, 2018

Automation for azure-sdk-for-js

Encountered an unknown error: (azure-sdk-for-js)

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 33, in exception_to_github
    yield context
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 170, in rest_handle_action
    return rest_pull_close(body, restapi_repo, sdk_pr_target_repo, sdkbase, sdk_tag)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 185, in rest_pull_close
    rest_pr_management(rest_pr, sdk_pr_target_repo, sdk_tag, sdk_default_base)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github_handler.py", line 151, in rest_pr_management
    sdk_tag=sdk_tag
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/SwaggerToSdkNewCLI.py", line 309, in generate_sdk_from_git_object
    sdk_repo.git.push('origin', base_branch, set_upstream=True)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 548, in <lambda>
    return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 1014, in _call_process
    return self.execute(call, **exec_kwargs)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 825, in execute
    raise GitCommandError(command, status, stderr_value, stdout_value)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git push --set-upstream origin restapi_auto_web/resource-manager
  stderr: 'remote: Permission to Azure/azure-sdk-for-js.git denied to AutorestCI.
fatal: unable to access 'https://AutorestCI:58ab395c311d1bd75b3e1db1cc8adaf9acc42afe@github.com/Azure/azure-sdk-for-js.git/': The requested URL returned error: 403'

@AutorestCI
Copy link

AutorestCI commented Oct 29, 2018

Automation for azure-sdk-for-ruby

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

@AutorestCI
Copy link

AutorestCI commented Oct 29, 2018

Automation for azure-sdk-for-go

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-go#4155

@AutorestCI
Copy link

AutorestCI commented Oct 29, 2018

Automation for azure-sdk-for-python

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-python#4429

@AutorestCI
Copy link

AutorestCI commented Oct 29, 2018

Automation for azure-sdk-for-java

Encountered an unknown error: (azure-sdk-for-java)

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 33, in exception_to_github
    yield context
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 170, in rest_handle_action
    return rest_pull_close(body, restapi_repo, sdk_pr_target_repo, sdkbase, sdk_tag)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 185, in rest_pull_close
    rest_pr_management(rest_pr, sdk_pr_target_repo, sdk_tag, sdk_default_base)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github_handler.py", line 151, in rest_pr_management
    sdk_tag=sdk_tag
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/SwaggerToSdkNewCLI.py", line 309, in generate_sdk_from_git_object
    sdk_repo.git.push('origin', base_branch, set_upstream=True)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 548, in <lambda>
    return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 1014, in _call_process
    return self.execute(call, **exec_kwargs)
  File "/usr/local/lib/python3.6/dist-packages/git/cmd.py", line 825, in execute
    raise GitCommandError(command, status, stderr_value, stdout_value)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git push --set-upstream origin restapi_auto_web/resource-manager
  stderr: 'remote: Permission to Azure/azure-sdk-for-java.git denied to AutorestCI.
fatal: unable to access 'https://AutorestCI:58ab395c311d1bd75b3e1db1cc8adaf9acc42afe@github.com/Azure/azure-sdk-for-java.git/': The requested URL returned error: 403'

@AutorestCI
Copy link

AutorestCI commented Oct 29, 2018

Automation for azure-sdk-for-node

A PR has been created for you:
Azure/azure-sdk-for-node#4757

@annatisch
Copy link
Member

Thanks for the fix @pietro!
The changes look good - however there are quite a few model validation errors for these two specs. Most of them are unrelated to your changes - however it would be great if you could fix up the errors related to the two samples you've updated (these 3 errors apply to the same sample in both API versions):

error : 
operationId: AppServicePlans_CreateOrUpdate
scenario: Create or Update App Service plan
source: response
responseCode: '200'
errorDetails:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - - mdmId
      - currentNumberOfWorkers
      - currentWorkerSize
      - numberOfWorkers
      - workerSize
  message: >-
    Additional properties not allowed:
    mdmId,currentNumberOfWorkers,currentWorkerSize,numberOfWorkers,workerSize
  path: properties
  jsonUrl: >-
    specification/web/resource-manager/Microsoft.Web/stable/2016-09-01/examples/CreateOrUpdateAppServicePlan.json

error : 
operationId: AppServicePlans_CreateOrUpdate
scenario: Create or Update App Service plan
source: response
responseCode: '201'
errorDetails:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - - mdmId
      - currentNumberOfWorkers
      - currentWorkerSize
      - numberOfWorkers
      - workerSize
  message: >-
    Additional properties not allowed:
    mdmId,currentNumberOfWorkers,currentWorkerSize,numberOfWorkers,workerSize
  path: properties
  jsonUrl: >-
    specification/web/resource-manager/Microsoft.Web/stable/2016-09-01/examples/CreateOrUpdateAppServicePlan.json

 error : 
operationId: AppServicePlans_CreateOrUpdate
scenario: Create or Update App Service plan
source: response
responseCode: '202'
errorDetails:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - - mdmId
      - currentNumberOfWorkers
      - currentWorkerSize
      - numberOfWorkers
      - workerSize
  message: >-
    Additional properties not allowed:
    mdmId,currentNumberOfWorkers,currentWorkerSize,numberOfWorkers,workerSize
  path: properties
  jsonUrl: >-
    specification/web/resource-manager/Microsoft.Web/stable/2016-09-01/examples/CreateOrUpdateAppServicePlan.json

If you felt like knocking out any of the other remaining example validation errors that would be bonus brownie points ;)
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/448024117

@pietro
Copy link
Contributor Author

pietro commented Oct 31, 2018

@annatisch Thanks for the review. I'll look into the errors and I'll try to update the models to match the examples.

@annatisch
Copy link
Member

Thanks @pietro - but please confirm whether it's the models or the examples that need to be updated.

@pietro
Copy link
Contributor Author

pietro commented Oct 31, 2018

@annatisch the models need to be updated. I'm adding the properties and their types, but I'm skipping the description.

@annatisch
Copy link
Member

@pietro - Please do not skip the description - even if it is simplistic for now. These descriptions are used to generate the API documentation for docs.microsoft.com and leaving them blank is confusing for customers.

Additionally - I see you are not yet a member of the Azure GitHub org - can you please join?

@pietro
Copy link
Contributor Author

pietro commented Nov 1, 2018

Please do not skip the description - even if it is simplistic for now

I'll add them soon.

I see you are not yet a member of the Azure GitHub org - can you please join?

Sure, can you send me the invite?

@annatisch
Copy link
Member

Thanks @pietro - please join through aka.ms/azuregithub :)

@pietro
Copy link
Contributor Author

pietro commented Nov 1, 2018

@annatisch I tried to join but I got this error:

Selected user account does not exist in tenant 'Microsoft' and cannot access the application 'SOME_UUID' in that tenant. The account needs to be added as an external user in the tenant first. Please use a different account.

Maybe I can't join because I'm not a MSFT employee?

@annatisch
Copy link
Member

Ah - sorry @pietro - I assumed you were with MSFT.
In that case I will need to bring in the service team to validate these PR changes.

@amarzavery
Copy link
Contributor

@naveedaz - Can you please review the changes in the PR? They are from an external contributor.

@sarangan12
Copy link
Member

@naveedaz - Can you please review the changes in the PR? They are from an external contributor.

1 similar comment
@sarangan12
Copy link
Member

@naveedaz - Can you please review the changes in the PR? They are from an external contributor.

@@ -371,6 +371,32 @@
"name": "ProvisioningState",
"modelAsString": false
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though these show up on the wire, they are legacy properties and need to be excluded from swagger. API clients should not depend on these properties. The validations fail currently and we are working with the Azure SDK team to suppress these validation errors. Instead please remove them from the examples.

@@ -410,6 +410,32 @@
"name": "ProvisioningState",
"modelAsString": false
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though these show up on the wire, they are legacy properties and need to be excluded from swagger. API clients should not depend on these properties. The validations fail currently and we are working with the Azure SDK team to suppress these validation errors. Instead please remove them from the examples.

@pietro
Copy link
Contributor Author

pietro commented Jan 17, 2019

@naveedaz I removed the commits that added the legacy properties.

@sarangan12
Copy link
Member

@naveedaz What is the plan for this PR? Are you planning to approve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants