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

Fix LRO PATCH and simplification #993

Closed
amarzavery opened this issue Apr 30, 2016 · 6 comments
Closed

Fix LRO PATCH and simplification #993

amarzavery opened this issue Apr 30, 2016 · 6 comments
Labels

Comments

@amarzavery
Copy link
Contributor

This PR #875 fixed LRO PATCH in node.js and also simplified it. Similar changes need to be made in C#, Java, Ruby, Python so that every language is consistent.

@tbombach
Copy link
Member

tbombach commented May 3, 2016

Thanks for opening this. I'm working on the C# changes

@annatisch
Copy link
Member

@amarzavery - are you able to clarify how PATCH operations were fixed? I'm having trouble finding it amongst the refactor changes in the PR diff....

@amarzavery
Copy link
Contributor Author

amarzavery commented May 5, 2016

@annatisch - sure.

Previously implemented long running PATCH was incorrectly giving preference to provisioning state value.

For a PATCH, the resource has to be successfully created as a prerequisite. Hence, its provisioning state will always be succeeded. We were checking that and would return without polling.

The right thing to do for a PATCH operation is to look for Azure Async Operation Header and Location Header in the initial response. If they are present then we should poll (get) on the link provided in those headers until a terminal state is reached.

We were doing this kind of polling for long running POST and DELETE. Hence, we modified the logic for PATCH to not look for provisioning state and do the right thing.

As a part of this modification we realized that everything can be simplified into one method named getLongRunningOperationStatus() (by checking the initial method and the statuscode) rather than having the previous two methods (getPutOrPatch.., getPostOrDelete..). Hence this was reduced to one.

We still kept the old methods for backwards compatibility. These methods internally call the new method.

Hope this helps :).

@vishrutshah
Copy link
Contributor

Removing Ruby Label as the Fix is in.

@annatisch
Copy link
Member

@amarzavery, thanks so much - that helped lots!!
I think I've finally got this resolved for Python - I'd appreciate it if you could take a quick glance over my latest PR.

@annatisch annatisch removed the Python label May 25, 2016
@jianghaolu
Copy link
Contributor

(Not for Java runtime GA as this won't be a breaking change. Will fix it later.)

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

No branches or pull requests

6 participants