Skip to content

{Core} Fix 404 in long running delete operation #15884

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

Merged
yungezz merged 3 commits into from
Apr 22, 2021
Merged

{Core} Fix 404 in long running delete operation #15884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef799b4
fix 404 in LRO
fengzhou-msft Nov 12, 2020
0070732
Merge branch 'dev' into fix_delete_lro
fengzhou-msft Feb 8, 2021
059532d
Merge branch 'dev' into fix_delete_lro
fengzhou-msft Apr 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/azure-cli-core/azure/cli/core/commands/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,11 +987,15 @@ def __call__(self, poller):
except ClientException 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']:
Copy link
Member

@jiasli jiasli Feb 8, 2021

Choose a reason for hiding this comment

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

Even if we add this hack, Python SDK and its users will still suffers from this non-standard behavior.

Copy link
Member

@jiasli jiasli Feb 8, 2021

Choose a reason for hiding this comment

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

Do we have an open issue in https://github.com/Azure/azure-rest-api-specs to track this?

Copy link
Member Author

@fengzhou-msft fengzhou-msft Feb 8, 2021

Choose a reason for hiding this comment

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

I opened an issue in our repo #15906 for service team to deal with.

Copy link
Member Author

@fengzhou-msft fengzhou-msft Feb 8, 2021

Choose a reason for hiding this comment

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

Based on a disucssion with swagger reviewers, there are some existing services using 404 in LRO. For new swaggers, there should be validation tasks to catch it but for existing ones as long as the json is not modified with PRs, it will not be caught.

Copy link
Member

@jiasli jiasli Feb 8, 2021

Choose a reason for hiding this comment

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

I still think we should force service team to fix it, instead of patching it from our side, otherwise Python SDK and its users will still suffers from this non-standard behavior.

logger.debug('Service returned 404 on the long-running delete operation. CLI treats it as delete '
'successfully but service should fix this behavior.')
return None
handle_long_running_operation_exception(client_exception)

self.progress_bar.end()
if poll_flag:
telemetry.poll_end()
finally:
self.progress_bar.end()
if poll_flag:
telemetry.poll_end()

return result

Expand Down
Loading