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

{Core} Work around long-running delete or purge operation with 404 status code #17833

Merged
merged 15 commits into from
Apr 23, 2021
Merged

{Core} Work around long-running delete or purge operation with 404 status code #17833

merged 15 commits into from
Apr 23, 2021

Conversation

houk-ms
Copy link
Contributor

@houk-ms houk-ms commented Apr 23, 2021

Description

This PR works around the Track2 case of the long-running delete or purge operation with 404 status code when polling completes.

Track1 works around it in this PR #15884

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


This checklist is used to make sure that common guidelines for a pull request are followed.

@houk-ms houk-ms self-assigned this Apr 23, 2021
@houk-ms houk-ms requested review from jsntcy and Juliehzl as code owners April 23, 2021 06:38
@houk-ms houk-ms changed the title {Core} Fix long-running delete or purge operation with 404 status code {Core} Work around long-running delete or purge operation with 404 status code Apr 23, 2021
Comment on lines 988 to 996
except (ClientException, HttpResponseError) as client_exception:
from azure.cli.core.commands.arm import handle_long_running_operation_exception
self.progress_bar.stop()
if getattr(client_exception, 'status_code', None) == 404 and 'delete' in self.cli_ctx.data['command']:
logger.debug('Service returned 404 on the long-running delete operation. CLI treats it as delete '
'successfully but service should fix this behavior.')
if getattr(client_exception, 'status_code', None) == 404 and \
('delete' in self.cli_ctx.data['command'] or 'purge' in self.cli_ctx.data['command']):
logger.debug('Service returned 404 on the long-running delete or purge operation. CLI treats it as '
'delete or purge successfully but service should fix this behavior.')
return None
handle_long_running_operation_exception(client_exception)
Copy link
Member

@fengzhou-msft fengzhou-msft Apr 23, 2021

Choose a reason for hiding this comment

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

Please double check whether handle_long_running_operation_exception can handle track 2 HttpResponseError as well.

Copy link
Contributor Author

@houk-ms houk-ms Apr 23, 2021

Choose a reason for hiding this comment

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

Yes, it could.
Before this change, the HttpResponseError is raised to an outer exception handler.
After this change, it just wraps the exception and throw a CLIError, which i think doesn't have risks here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may add anthor except block to keep the exactly same behavior, but i think it's not really necessary.

@houk-ms
Copy link
Contributor Author

houk-ms commented Apr 23, 2021

Test cases will fail if we wrap the error in advance rather than raise it. So i just update the codes to keep the original behavior.

@houk-ms houk-ms merged commit 993537f into Azure:dev Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core CLI core infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants