Skip to content

blanket try-catch to ensure following unpatch code#6385

Merged
williexu merged 3 commits into
Azure:devfrom
williexu:parser_log
May 18, 2018
Merged

blanket try-catch to ensure following unpatch code#6385
williexu merged 3 commits into
Azure:devfrom
williexu:parser_log

Conversation

@williexu
Copy link
Copy Markdown
Contributor


Following from feedback on #6381

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

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@promptws
Copy link
Copy Markdown

View a preview at https://prompt.ws/r/Azure/azure-cli/6385
This is an experimental preview for @microsoft users.

@williexu williexu requested a review from tjprescott May 17, 2018 22:50
parse_args = self.argsfinder.get_parsed_args(
parse_quotes(text, quotes=False, string=False))
try:
parse_args = self.argsfinder.get_parsed_args(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same line.

try:
parse_args = self.argsfinder.get_parsed_args(
parse_quotes(text, quotes=False, string=False))
except Exception: # pylint: disable=broad-except
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What kind the exception will actually be thrown here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

From what I can see, none. argparse will call error() (which is also patched) with any parser related errors.
This is more of a safety assurance change- we don't want the patched methods to persist anywhere beyond this scope.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add comments in code.

@troydai troydai added this to the Sprint 37 milestone May 17, 2018
try:
parse_args = self.argsfinder.get_parsed_args(
parse_quotes(text, quotes=False, string=False))
except Exception: # pylint: disable=broad-except
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add comments in code.

@williexu
Copy link
Copy Markdown
Contributor Author

@troydai addressed your comments

@troydai
Copy link
Copy Markdown
Contributor

troydai commented May 17, 2018

Merge once CI passes.

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 18, 2018

Codecov Report

Merging #6385 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##           dev   #6385   +/-   ##
===================================
  Coverage    0%      0%           
===================================
  Files       11      11           
  Lines      133     133           
  Branches     8       9    +1     
===================================
  Misses     133     133

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 301ddd7...cb85b4f. Read the comment docs.

@williexu williexu merged commit 6d8dbb0 into Azure:dev May 18, 2018
@williexu williexu deleted the parser_log branch May 18, 2018 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants