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.

Sign up for GitHub

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

Jump to bottom

Add Network Profile swagger and examples #3806

Merged
merged 3 commits into from
Sep 7, 2018
Merged

Add Network Profile swagger and examples #3806

merged 3 commits into from
Sep 7, 2018

Conversation

Flip-Liquid
Copy link
Contributor

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

  • The title of the PR 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 information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Sep 5, 2018 •
edited
Loading

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI AutorestCI mentioned this pull request Sep 5, 2018
Merged
@AutorestCI
Copy link

AutorestCI commented Sep 5, 2018 •
edited
Loading

Automation for azure-sdk-for-python

A PR has been created for you:
Azure/azure-sdk-for-python#3269

@azuresdkci azuresdkci requested a review from lmazuel September 5, 2018 11:35
@AutorestCI
Copy link

AutorestCI commented Sep 5, 2018 •
edited
Loading

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@AutorestCI
Copy link

AutorestCI commented Sep 5, 2018 •
edited
Loading

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI AutorestCI mentioned this pull request Sep 5, 2018
Merged
@AutorestCI
Copy link

AutorestCI commented Sep 5, 2018 •
edited
Loading

Automation for azure-sdk-for-node

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

@Flip-Liquid
Copy link
Contributor Author

@jianghaolu , would you mind reviewing as well? Unable to request review since I don't have write access on the repo :)

@lmazuel lmazuel added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Sep 5, 2018
@lmazuel lmazuel requested a review from ravbhatnagar September 5, 2018 16:50
@lmazuel
Copy link
Member

lmazuel commented Sep 5, 2018

@ravbhatnagar from ARM as well for review

lmazuel
lmazuel reviewed Sep 5, 2018
View reviewed changes
specification/network/resource-manager/Microsoft.Network/stable/2018-08-01/networkProfile.json Outdated
"Container": {
"properties": {
"properties": {
"$ref": "#/definitions/ContainerPropertiesFormat",
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to use x-ms-client-flatten to make better SDK.
Example:

azure-rest-api-specs/specification/network/resource-manager/Microsoft.Network/stable/2016-12-01/applicationGateway.json

Line 511 in 32b0d87

"x-ms-client-flatten": true,

lmazuel
lmazuel reviewed Sep 5, 2018
View reviewed changes
specification/network/resource-manager/Microsoft.Network/stable/2018-08-01/networkProfile.json Outdated
"IPConfigurationProfile": {
"properties": {
"properties": {
"$ref": "#/definitions/IPConfigurationProfilePropertiesFormat",
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to use x-ms-client-flatten to make better SDK.
Example:

azure-rest-api-specs/specification/network/resource-manager/Microsoft.Network/stable/2016-12-01/applicationGateway.json

Line 511 in 32b0d87

"x-ms-client-flatten": true,

lmazuel
lmazuel reviewed Sep 5, 2018
View reviewed changes
specification/network/resource-manager/Microsoft.Network/stable/2018-08-01/networkProfile.json Outdated
"ContainerNetworkInterfaceConfiguration": {
"properties": {
"properties": {
"$ref": "#/definitions/ContainerNetworkInterfaceConfigurationPropertiesFormat",
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to use x-ms-client-flatten to make better SDK.
Example:

azure-rest-api-specs/specification/network/resource-manager/Microsoft.Network/stable/2016-12-01/applicationGateway.json

Line 511 in 32b0d87

"x-ms-client-flatten": true,

lmazuel
lmazuel reviewed Sep 5, 2018
View reviewed changes
specification/network/resource-manager/Microsoft.Network/stable/2018-08-01/networkProfile.json Outdated
"ContainerNetworkInterface": {
"properties": {
"properties": {
"$ref": "#/definitions/ContainerNetworkInterfacePropertiesFormat",
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to use x-ms-client-flatten to make better SDK.
Example:

azure-rest-api-specs/specification/network/resource-manager/Microsoft.Network/stable/2016-12-01/applicationGateway.json

Line 511 in 32b0d87

"x-ms-client-flatten": true,

lmazuel
lmazuel reviewed Sep 5, 2018
View reviewed changes
specification/network/resource-manager/Microsoft.Network/stable/2018-08-01/networkProfile.json Outdated
"NetworkProfile": {
"properties": {
"properties": {
"$ref": "#/definitions/NetworkProfilePropertiesFormat",
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to use x-ms-client-flatten to make better SDK.
Example:

azure-rest-api-specs/specification/network/resource-manager/Microsoft.Network/stable/2016-12-01/applicationGateway.json

Line 511 in 32b0d87

"x-ms-client-flatten": true,

lmazuel
lmazuel approved these changes Sep 5, 2018
View reviewed changes
Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

Sounds good. A few remark about flatenning, this would avoid myvariable.properties.myname to be simply myvariable.myname

tjprescott
tjprescott requested changes Sep 5, 2018
View reviewed changes
Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

Please implement @lmazuel's flattening suggestions. Without them, then Python SDK is a mess.

@Flip-Liquid Flip-Liquid force-pushed the Network-September-Release branch from 372a367 to 6880c21 Compare September 5, 2018 20:24
@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Sep 6, 2018
@ravbhatnagar
Copy link
Contributor

This was reviewed in person. Signing off!

@lmazuel
Copy link
Member

lmazuel commented Sep 6, 2018

Hi @jcgknudson
Please also fix the model validation errors:
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/424973532

This could mean that the examples are wrong, or the Swagger is wrong. Be sure that the example reflects the exact truth of the service. Once it's done, if there is still issues, this means your Swagger is wrong (because if example tells the truth, then it should match).
Let me know if something is unclear.
Thank you!

@Flip-Liquid
Copy link
Contributor Author

Hey @lmazuel, fixed the issues with the model validation, sorry about that.
There were a few fields omitted in the model, nothing too big.

@lmazuel lmazuel merged commit cc47342 into Azure:Network-September-Release Sep 7, 2018
dsgouda pushed a commit that referenced this pull request Sep 11, 2018
@MikhailTryakhov @dsgouda
Local september branch merge to master (#3862)
42006c5 
* Trusted Root certificate (#3668)

* Rebased master to monthly network branch (#3687)

* Swagger Coverage/Completeness- for operation CheckNameAvailability (#3663)

* Update service.json

* CheckNameAvailability

* FIxed Parameter name

fixed couple of parameters name

* Update dns management Node.js version number to 3.0.0 (#3675)

* Update batchai Node.js package version to 2.0.2 (#3664)

* Update nofiticationhubs management Node.js package version to 2.0.0 (#3665)

* WebApps - Add BackupName to backup API models (#3560)

* WebApps - Add BackupName to replace non-ARM-compliant Name property on Backup API models

* Revert breaking change to BackupItem. BackupItemName property will be fine in place of BackupName for now.

* Update authorization management Node.js package version to 5.0.1 (#3676)

* Update storagesync management Node.js package version to 1.0.0 (#3677)

* add checknameavailability (#3608)

* add checknameavailability

* move checkname parameter to definition section

* fix brackets

* fix parameter name in checknameavailability exanple

* update code owners for PowerBIDedicated

* New Batch data plane API version 2018-08-01.7.0 (#3657)

* Add new batch version 2018-08-01.7.0

* Update the new API

* Fix up output from OAD, it does not output valid JSON

* update OAV (#3678)

* Update storage management Node.js package version to 5.2.0 (#3679)

* Update compute.json and disk.json for some issues and add zone and rolling upgrade. (#3642)

* Breaking change tool correctly handles multiple files with same name

* Fix to breaking changes table generation

* Trusted Root certificate (#3668)

* Updated Microsoft.Network version to 2018-08-01 (#3672)

* Adds base for updating Microsoft.Network from version stable/2018-07-01 to version 2018-08-01

* Updates readme

* Updates API version in new specs and examples

* Added AddressPrefixes on Subnet properties (#3714)

* Added address prefixes to subnet

* Added example of multiple address prefixes.

* Python Network conf 2018-08-01

* Changes to include required properties for outbound rule (#3728)

* Mark outbound rule properties as required

* Update outbound rule with required properties

* Removing old change

* Fix Python 2018-08 conf

* Use only PublicIpAddress in Azure Firewall, not both Public and InternalPublic (#3743)

* Do not confuse users with both InternalPublicIp and PublicIp for the same thing

* Update AzureFirewall PUT example

* This closes #3474

* Use the correct name for publicIPAddress in AzureFirewall PUT example

* Azure Firewall FQDN Tag top level resource (#3744)

* Azure Firewall FQDN Tag top level resource

* Name FqdnTags property in AzureFirewallApplicationRule properly

* Empty commit to trigger new validation

* Vtapfinal (#3684)

* Swagger Coverage/Completeness- for operation CheckNameAvailability (#3663)

* Update service.json

* CheckNameAvailability

* FIxed Parameter name

fixed couple of parameters name

* Adds base for updating Microsoft.Network from version stable/2018-07-01 to version 2018-08-01

* Updates readme

* Updates API version in new specs and examples

* vtap update

* VTAP changes

* VTAP changes

* Adding examples for VTAP resource

* Update changes for Examples

* Further fixes to examples

* Add exception for vtap's RequiredPropertiesMissingInResourceModel in readme

* Azure Firewall NAT Rule Collection (#3745)

* NAT Rule Collections for Azure Firewall

* Update examples to include the new NAT Rule Collection

* Shorten the NAT RC Properties name and mark etag read-only

* Add NAT Rule Collection to Azure Firewall properties

* Fix all <<a Azure>> in helper messages

* Do not allow ICMP as Network protocol (#3742)

* Adding linkedResourceType and hostedWorkloads to nic (#3780)

* Adding support for interface endpoints (#3783)

* Creating Interface Endpoint definition

* Adding references to interface endpoints in existing resources

* Added Trusted root certificate changes to new API version (#3731)

* Add ExpressRoute gateway resource (#3776)

* Initial version

* Refactor connections.

* Update API version

* Add enum values.

* Addressed all comments.

* Adding support for delegations on a subnet (#3805)

* Adding subnet delegation property on VNET

* Updating subnet/vnet create & get examples to reflect delegation

* Adding support for GetAvailableDelegations operation

* Fixing properties in Swagger & updating examples to reflect fixes

* Add availableDelegation as a subResource

* Remove type from examples

* Further updates to delegation PR

* Additional changes to structure

* Correct issues for SDK generation (#3818)

* Remove x-ms-client-flatten attribute as that results in wrong
descriptiosn during generation of the SDKs.

* Missing descriptions.

* Rename TargetUrls to TargetFqdns (#3739)

* Add Network Profile swagger and examples (#3806)

* add network profile swagger and examples

* add x-ms-client-flatten property as per feedback

* appease travis by fixing model + examples

* vtapswaggerupdate (#3837)

* Add Service Association Link and IP Configuration Profiles on Subnet (#3840)

* add service association links for virtual network

* add ipconfig profiles on subnet

* fixed example issue

* Review for tabfix and properties fromat rename

* Vtap swagger update and made the property as readonly (#3845)

* vtapswaggerupdate

* Adding readonly property

* Updated AppGW Autoscale Configuration (#3838)

* VPN gateway new API for Reset VPN client PSK (#3800)

* Brooklyn Ignite Swagger changes (#3715)

* Brooklyn Ignite Swagger changes

* Add missing example file changes

* Add virtual hub route table

* Fix travis build error

* VPN profile bugfix

* Fix casing for NRP automapping

* Fix parsing errors

* remove BgpSettings from P2SVpnGateway object

* Added AddressPrefixes on Subnet properties (#3714)

* Added address prefixes to subnet

* Added example of multiple address prefixes.

* Python Network conf 2018-08-01

* Changes to include required properties for outbound rule (#3728)

* Mark outbound rule properties as required

* Update outbound rule with required properties

* Removing old change

* Fix Python 2018-08 conf

* Fix swagger contract errors with NRP

* Fix network swagger

* Fix virtualHub Properties for SDK and PS JSON serializer compliance

* Fix bugs

* Changing Cortex swagger to match NRP

* Bug fixes

* bugFix3

* Bug fix

* Use only PublicIpAddress in Azure Firewall, not both Public and InternalPublic (#3743)

* Do not confuse users with both InternalPublicIp and PublicIp for the same thing

* Update AzureFirewall PUT example

* This closes #3474

* Use the correct name for publicIPAddress in AzureFirewall PUT example

* Azure Firewall FQDN Tag top level resource (#3744)

* Azure Firewall FQDN Tag top level resource

* Name FqdnTags property in AzureFirewallApplicationRule properly

* Empty commit to trigger new validation

* Vtapfinal (#3684)

* Swagger Coverage/Completeness- for operation CheckNameAvailability (#3663)

* Update service.json

* CheckNameAvailability

* FIxed Parameter name

fixed couple of parameters name

* Adds base for updating Microsoft.Network from version stable/2018-07-01 to version 2018-08-01

* Updates readme

* Updates API version in new specs and examples

* vtap update

* VTAP changes

* VTAP changes

* Adding examples for VTAP resource

* Update changes for Examples

* Further fixes to examples

* Add exception for vtap's RequiredPropertiesMissingInResourceModel in readme

* Azure Firewall NAT Rule Collection (#3745)

* NAT Rule Collections for Azure Firewall

* Update examples to include the new NAT Rule Collection

* Shorten the NAT RC Properties name and mark etag read-only

* Add NAT Rule Collection to Azure Firewall properties

* Fix all <<a Azure>> in helper messages

* Do not allow ICMP as Network protocol (#3742)

* Adding linkedResourceType and hostedWorkloads to nic (#3780)

* Adding support for interface endpoints (#3783)

* Creating Interface Endpoint definition

* Adding references to interface endpoints in existing resources

* Added Trusted root certificate changes to new API version (#3731)

* Add ExpressRoute gateway resource (#3776)

* Initial version

* Refactor connections.

* Update API version

* Add enum values.

* Addressed all comments.

* Adding support for delegations on a subnet (#3805)

* Adding subnet delegation property on VNET

* Updating subnet/vnet create & get examples to reflect delegation

* Adding support for GetAvailableDelegations operation

* Fixing properties in Swagger & updating examples to reflect fixes

* Add availableDelegation as a subResource

* Remove type from examples

* Further updates to delegation PR

* Additional changes to structure

* Correct issues for SDK generation (#3818)

* Remove x-ms-client-flatten attribute as that results in wrong
descriptiosn during generation of the SDKs.

* Missing descriptions.

* Rename TargetUrls to TargetFqdns (#3739)

* Fix travis errors in examples

* Fixed error examples for P2S cortex resources

* Fix swagger

* Fix Semantic validation error

* network swagger fix

* Fix travis

* Fix travis errors

* Revert version change

* Dummy changes for CI restart

* Swagger changes (#3857)

* Relocated readOnly property

* Readded provisioning state to interfaceEndpoint.json

* Updated networkInterface.json

* Updated serviceEndpointPolicy.json

* Update virtualNetworkTap.json

* Fix semantic issue

* Multiple changes

* More example fixes

* Dummy commit

* Fixed duplicate

* changes to swagger in interface endpoints and subnet delegation (#3860)

* Revert "Trusted Root certificate (#3668)"

This reverts commit 3ff4b52.

* fixed api version in azure firewall example

* Review comments incorporation (#3859)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjprescott tjprescott tjprescott approved these changes

@lmazuel lmazuel lmazuel approved these changes

@MikhailTryakhov MikhailTryakhov Awaiting requested review from MikhailTryakhov

@ravbhatnagar ravbhatnagar Awaiting requested review from ravbhatnagar

Assignees

@lmazuel lmazuel

Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Flip-Liquid @azuresdkci @AutorestCI @lmazuel @ravbhatnagar @tjprescott