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

Moving sfrp release 2019-03-01 to stable #6613

Merged

anuchandy merged 8 commits into Azure:master from a-santamaria:sfrp_release_2019-03-01

Aug 7, 2019

Member

a-santamaria commented Jul 12, 2019 •

edited

Loading

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.


          moving release 2019-03-01 to stable

50861d1

a-santamaria requested a review from samedder as a code owner

July 12, 2019 23:03

azuresdkci assigned anuchandy

azuresdkci requested a review from anuchandy

July 12, 2019 23:04

AutorestCI mentioned this pull request

[AutoPR servicefabric/resource-manager] Moving sfrp release 2019-03-01 to stable Azure/azure-sdk-for-python#6340

Merged

AutorestCI commented Jul 12, 2019 •

edited

Loading

Automation for azure-sdk-for-python

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

AutorestCI commented Jul 12, 2019 •

edited

Loading

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

Contributor

azuresdkci commented Jul 12, 2019

Can one of the admins verify this patch?

AutorestCI mentioned this pull request

[AutoPR servicefabric/resource-manager] Moving sfrp release 2019-03-01 to stable Azure/azure-sdk-for-go#5244

Closed

AutorestCI commented Jul 12, 2019 •

edited

Loading

Automation for azure-sdk-for-go

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

a-santamaria closed this


          remove examples_new folder and fix examples

62ba0d7

a-santamaria reopened this

a-santamaria closed this

a-santamaria reopened this

mchiriac-msft approved these changes

View reviewed changes

Member Author

a-santamaria commented Jul 17, 2019

@anuchandy could you help review the pr?

anuchandy added the WaitForARMFeedback label

Member

anuchandy commented Jul 18, 2019

@a-santamaria is there any change other than changing api-version string in swagger from preview to stable? any models changed, operations added/removed?

Member Author

a-santamaria commented Jul 18, 2019

@anuchandy no, we only moved it to stable

KrisBash suggested changes

View reviewed changes

Contributor

KrisBash left a comment

It seems applications swagger has not been ARM reviewed previously, and has some resource model issues.

...cation/servicefabric/resource-manager/Microsoft.ServiceFabric/stable/2019-03-01/cluster.json

+                        "type": "string",
+                        "description": "The Service Fabric runtime version of the cluster. This property can only by set the user when **upgradeMode** is set to 'Manual'. To get list of available Service Fabric versions for new clusters use [ClusterVersion API](./ClusterVersion.md). To get the list of available version for existing clusters use **availableClusterVersions**."
+                      },
+                      "eventStoreServiceEnabled": {

Contributor

KrisBash Jul 19, 2019

Rather than a bool, typing as a string enum (with modelAsString:true) enables you to more easily add enum values in the future if you find you need more than binary options. I.e. eventServiceSetting: [enabled, disabled]

Member Author

a-santamaria Jul 22, 2019

thanks for the suggestion, we're going to leave as bool for this particular parameter.

...on/servicefabric/resource-manager/Microsoft.ServiceFabric/stable/2019-03-01/application.json

+                      }
+                    }
+                  },
+                  "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ServiceFabric/clusters/{clusterName}/applications/{applicationName}": {

Contributor

KrisBash Jul 19, 2019

Something that confuses me: in your manifest, "applications" (top-level resource) is defined as a tracked resource, but "clusters/applications" (this one) is defined as a proxy resource. The swagger schema for "clusters/applications" models it as a proxy resource. Is this supposed to be tracked or proxy? Do you support discrete location values or clusters and applications? What is the top-level "applications" resource in the manifest if it is not in the swagger?

Member Author

a-santamaria Jul 22, 2019 •

edited

Loading

fixed by removing location and tags properties from proxyResource

...on/servicefabric/resource-manager/Microsoft.ServiceFabric/stable/2019-03-01/application.json

+                      },
+                      "x-ms-long-running-operation": true,
+                      "responses": {
+                        "202": {

Contributor

KrisBash Jul 19, 2019

Is this always async, or is there ever a noop/200 response?

Member Author

a-santamaria Jul 22, 2019

yes, always async

...on/servicefabric/resource-manager/Microsoft.ServiceFabric/stable/2019-03-01/application.json

+                        }
+                      }
+                    },
+                    "patch": {

Contributor

KrisBash Jul 19, 2019

/clusters/applications/services is not currently defined in the manifest, but is modeled in swagger as a tracked resource - with location and tags properties, and a PATCH api for tags. It is unusual to have a tracked resource as a nested/child resource, as the ancestors and child could have discrete locations. If this truly is intended to be a tracked resource, it also needs to be in the manifest. If it is proxy, remove tags, and location from the model -optionally you can remove this PATCH from a proxy resource (or at least remove tags from request body)

Member Author

a-santamaria Jul 22, 2019

fixed by removing location and tags properties from proxyResource

...on/servicefabric/resource-manager/Microsoft.ServiceFabric/stable/2019-03-01/application.json Show resolved Hide resolved

...on/servicefabric/resource-manager/Microsoft.ServiceFabric/stable/2019-03-01/application.json

+                      }
+                    }
+                  },
+                  "ProxyResource": {

Contributor

KrisBash Jul 19, 2019

This appears to be the root of the problem - "proxyResource" should not have location or tags properties.

Member Author

a-santamaria Jul 22, 2019

thanks for the feedback. Yes, you're right, removed

Member Author

a-santamaria Jul 22, 2019 •

edited

Loading

@KrisBash I've actually reverted this change because we would need to make some changes to our service for this. Can we keep it like this for this release? we will add a new version soon with these changes

...on/servicefabric/resource-manager/Microsoft.ServiceFabric/stable/2019-03-01/application.json

+                    "items": {
+                      "$ref": "#/definitions/ServicePlacementPolicyDescription"
+                    },
+                    "description": "A list that describes the correlation of the service wi

Contributor

KrisBash Jul 19, 2019

As mentioned previously, consider a string enum rather than a bool

Member Author

a-santamaria Jul 22, 2019

which property are you referring to in this case?

KrisBash added the ARMChangesRequested label


          remove location and tags from proxyResource

b1a5bbf

openapi-sdkautomation bot commented Jul 22, 2019 •

edited

Loading

In Testing, Please Ignore

[Logs] (Generated from 7ae9bfe, Iteration 6)

.NET: test-repo-billy/azure-sdk-for-net [Logs] [Diff]

Microsoft.Azure.Management.ServiceFabric [Logs] [Instructions]

Python: test-repo-billy/azure-sdk-for-python [Logs] [Diff]

azure-mgmt-servicefabric [Logs] [Instructions] [Generation PR]
- azure-mgmt-servicefabric-0.3.0.zip
- azure_mgmt_servicefabric-0.3.0-py2.py3-none-any.whl

Java: test-repo-billy/azure-sdk-for-java [Logs] [Diff]

servicefabric/resource-manager/v2016_09_01 [Logs]
servicefabric/resource-manager/v2017_07_01_preview [Logs]
servicefabric/resource-manager/v2018_02_01 [Logs]

Go: test-repo-billy/azure-sdk-for-go [Logs] [Diff]

preview/servicefabric/mgmt/2017-07-01-preview [Logs]
preview/servicefabric/mgmt/2019-03-01-preview [Logs] [Instructions] [Generation PR]
- preview.servicefabric.mgmt.2019-03-01-preview.zip
servicefabric/mgmt/2016-09-01 [Logs]
servicefabric/mgmt/2018-02-01 [Logs]
servicefabric/mgmt/2019-03-01 [Logs] [Instructions] [Generation PR]
- servicefabric.mgmt.2019-03-01.zip

Ruby: test-repo-billy/azure-sdk-for-ruby [Logs] [Diff]

No packages generated.

AutorestCI mentioned this pull request

[AutoPR servicefabric/resource-manager] Moving sfrp release 2019-03-01 to stable Azure/azure-sdk-for-go#5314

Merged

Alfredo Santamaria Gomez added 2 commits

July 22, 2019 11:11


          Revert "remove location and tags from proxyResource"

14c9abc

This reverts commit b1a5bbf.


          change location description in ProxyResource to dcument as deprecated

2380d7a

This was referenced Jul 29, 2019

[AutoPR servicefabric/resource-manager/v2016_09_01] Moving sfrp release 2019-03-01 to stable Azure/azure-sdk-for-java#4664

Closed

[AutoPR servicefabric/resource-manager/v2017_07_01_preview] Moving sfrp release 2019-03-01 to stable Azure/azure-sdk-for-java#4665

Closed

[AutoPR servicefabric/resource-manager/v2018_02_01] Moving sfrp release 2019-03-01 to stable Azure/azure-sdk-for-java#4666

Closed

KrisBash approved these changes

View reviewed changes

Contributor

KrisBash left a comment

Signing off from ARM, understanding that the spurious properties reflect existing server behavior and will require a new API version to remediate.

KrisBash added ARM-Review-Issue ARMSignedOff and removed ARMChangesRequested WaitForARMFeedback labels

Member Author

a-santamaria commented Aug 2, 2019 •

edited

Loading

@anuchandy can the pr be merged? as it's signed of by ARM

anuchandy approved these changes

View reviewed changes

Alfredo Santamaria Gomez added 3 commits

August 5, 2019 14:51


          rename put operations to *_CreateOrUpdate

17ffb08


          fix 2018-02 namespace in ruby.md

d67211b


          removing package-2018-02 from ruby (nto being used and its failing to…

7ae9bfe

… generate sdk)

anuchandy approved these changes

View reviewed changes

anuchandy merged commit 725048b into Azure:master

a-santamaria mentioned this pull request

service fabric sdk api version 2019-03-01 Azure/azure-sdk-for-net#7198

Merged

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

Reviewers

mchiriac-msft mchiriac-msft approved these changes

KrisBash KrisBash approved these changes

anuchandy anuchandy approved these changes

samedder Awaiting requested review from samedder samedder is a code owner

Labels