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

modeling assignmentOperations with api-version 2018-11-01 #4590

Merged
merged 32 commits into from
Jan 11, 2019
Merged

modeling assignmentOperations with api-version 2018-11-01 #4590

merged 32 commits into from
Jan 11, 2019

Conversation

haitch
Copy link
Member

@haitch haitch commented Nov 27, 2018

Changes

  • Modeling assignmentOperations, so SDK client can retrieve details about any assignment and history as well.

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.

@openapi-portal-comment
Copy link

If you're a MSFT employee, click this link
to view this PR's validation status on our new OpenAPI Hub spec management tool.

@AutorestCI
Copy link

AutorestCI commented Nov 27, 2018

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

@AutorestCI
Copy link

AutorestCI commented Nov 27, 2018

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented Nov 27, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Nov 27, 2018

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@AutorestCI
Copy link

AutorestCI commented Nov 27, 2018

Automation for azure-sdk-for-go

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

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@ravbhatnagar
Copy link
Contributor

As per discussion with Marcin over email, here is what will happen. tracking it here- #4903
/assignmentOperations
In a resource ID, it will look like this /subscriptions/{id}/providers/Microsoft.Blueprint/blueprintAssignments/{name}/assignmentOperations, which is redundant. On the upside the generated SDK already calls this object AssignmentOperations, so it will match the API now. The downside is that we will continue supporting /operations on API version 2017-11-11-. API versions 2018-11-01- and later would use /assignmentOperations. This will remain in our RP manifest and service code until 2017-11-11-preview is fully deprecated and removed. This seems like a good compromise to get this PR unblocked.

@ravbhatnagar
Copy link
Contributor

Signing off from ARM side.

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Dec 12, 2018
@haitch
Copy link
Member Author

haitch commented Dec 12, 2018

#4903 is addressed in latest commit

@haitch
Copy link
Member Author

haitch commented Dec 12, 2018

@annatisch can you take a look again and merge?

@annatisch
Copy link
Member

Thanks @haitch - model validation pass is still failing:

 error :
operationId: AssignmentOperations_List
errorDetails:
  code: DOUBLE_FORWARD_SLASHES_IN_URL
  message: >-
    In operation "AssignmentOperations_List", example for parameter "scope":
    "/subscriptions/f8df94f2-2f5a-4f4a-bcaf-1bb992fb564b" starts with a forward
    slash and the path template:
"/{scope}/providers/Microsoft.Blueprint/blueprintAssignments/{assignmentName}/assignmentOperations"    contains a forward slash before the parameter starts. This will cause double
    forward slashes  in the request url. Thus making it incorrect. Please
    rectify the example.

 error : 
operationId: AssignmentOperations_Get
errorDetails:
  code: DOUBLE_FORWARD_SLASHES_IN_URL
  message: >-
    In operation "AssignmentOperations_Get", example for parameter "scope":
    "/subscriptions/f8df94f2-2f5a-4f4a-bcaf-1bb992fb564b" starts with a forward
    slash and the path template:
"/{scope}/providers/Microsoft.Blueprint/blueprintAssignments/{assignmentName}/assignmentOperations/{assignmentOperationName}"
    contains a forward slash before the parameter starts. This will cause double
    forward slashes  in the request url. Thus making it incorrect. Please
    rectify the example.

@haitch
Copy link
Member Author

haitch commented Dec 13, 2018

fixed, @annatisch can you approve?

@annatisch
Copy link
Member

Thanks @haitch - I will try to review this afternoon

@annatisch
Copy link
Member

@haitch - is this API already deployed/available?

@haitch
Copy link
Member Author

haitch commented Dec 13, 2018

it was deployed with /operations last week.

I've also updated our RP with the rename, the rename deploy soon.

@annatisch
Copy link
Member

Thanks - please update this thread when the rename has been deployed (or when deployment has started).

Copy link
Member

@annatisch annatisch left a comment

Choose a reason for hiding this comment

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

Thanks @haitch - the PR looks good! :)
My only comments are minor regarding the consistent capitalization of the descriptions.

Copy link
Member

@annatisch annatisch left a comment

Choose a reason for hiding this comment

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

LGTM.
Waiting on service deployment to merge :)

@annatisch
Copy link
Member

@haitch - Is this API now public?

@haitch
Copy link
Member Author

haitch commented Jan 8, 2019

@annatisch it is public, you can safely merge this PR.

@annatisch
Copy link
Member

@RyanBensonMSFT - could you please sign off your review if your requested changes have been addressed?

Adding @lmazuel to merge PR once that sign off happens.

@filizt
Copy link
Contributor

filizt commented Jan 10, 2019

@RyanBensonMSFT please let us know if anything is blocking the sign off.

@filizt
Copy link
Contributor

filizt commented Jan 10, 2019

@lmazuel could you merge the PR please?

@lmazuel lmazuel merged commit 1b8809e into Azure:master Jan 11, 2019
TalluriAnusha pushed a commit to AsrOneSdk/azure-rest-api-specs that referenced this pull request Feb 6, 2019
* move blueprint from private repo to public

* address comment

* fat finger

* add description in ResourceProviderOperation

* camelCase for resourceType

* strongType ParameterValueCollection

* remove the hacky workaround for extension resource path

* init apiVersion=Nov2018

* api definition

* progress

* split assignment and blueprint definition

* assignmentOperation api

* split swagger to multiple area for easy upgrade

* update suppression due to file split

* modeling assignmentOperations

* suppress warnings on new file

* fix assignmentOperation list

* typo

* add python repo

* add title and description, fix old apiversion in examples

* update assignmentOperation use scope for more flexbility

* rename blueprintAssignments/operations to blueprintAssignments/assignmentOperations

* fix double forward slashes

* description update

* more capital letters

* werid casing error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

10 participants