-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Batch commands no longer throw error upon precondition failures. #5702
Conversation
View a preview at https://prompt.ws/r/Azure/azure-cli/5702 |
cdc49a0
to
5d5fe1e
Compare
|
||
|
||
def check_precondition_success(func): | ||
def new_func(*args, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrapper
def new_func(*args, **kwargs): | ||
from azure.common import AzureHttpError | ||
try: | ||
return (True, func(*args, **kwargs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need the ()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return 1, 2 # => (1,2)
return 1 => 1
except AzureHttpError as ex: | ||
# Precondition failed error | ||
if ex.status_code != 412: | ||
raise ex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can we log a warning here?
# Precondition failed error | ||
if ex.status_code != 412: | ||
raise ex | ||
return (False, None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need ()
except AzureHttpError as ex: | ||
# Precondition failed error | ||
if ex.status_code != 412: | ||
raise ex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can we log a warning here?
Looks good besides the comments. |
@troydai could you review my feedback changes? |
Closes: #5286
-batch commands should not fail upon preconditions set by user
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines