Summary

Remove CommandError usage from rbtools.utils

Review Request #9897 — Created April 30, 2018 and submitted June 4, 2018, 1:44 p.m.

Information

Owner

david

Repository

RBTools

Branch

release-1.0.x

Bugs

Depends On

Commit

7e54390...

Reviewers

Groups

rbtools

People

Description

CommandError is intended to be used only within rbtools.commands, and
it was a layering violation to use it from util code. I've swapped out
the places that raised it to instead use other, more appropriate
exception types, and then the commands that call those APIs can catch
and handle those.

Testing Done


Tested (most of) the various error conditions that caused the problems.
Ran unit tests.

Issues

Description	From	Last Updated
F821 undefined name 'six'	reviewbot	April 30, 2018, 11:15 a.m.
F821 undefined name 'six'	reviewbot	April 30, 2018, 11:15 a.m.
F821 undefined name 'review_request_id'	reviewbot	April 30, 2018, 11:15 a.m.
Can we use keyword arguments here, to help with readability?	chipx86	April 30, 2018, 6:20 p.m.
Maybe worth inversing this? if i == num_retries - 1: raise logging.error(...) Maybe getting into the weeds here, but that …	chipx86	April 30, 2018, 6:21 p.m.
There's no need for an else. The raise ends things for this function.	chipx86	May 4, 2018, 3:26 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/commands/post.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F821 undefined name 'six'
```
rbtools/commands/stamp.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F821 undefined name 'six'
```
rbtools/commands/stamp.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F821 undefined name 'review_request_id'
```

Commit:

16e3b5106424e833e768c74ef00158371f190283

2d08e972aed0d6934b4a3d4f4b0da95e62b8624a

Diff:

Revision 2 (+114 -79)

Show changes

	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/land.py
	rbtools/commands/login.py
	rbtools/commands/post.py
	rbtools/commands/publish.py
	rbtools/commands/stamp.py
	rbtools/utils/commands.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/utils/users.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Can we use keyword arguments here, to help with readability?
```

rbtools/utils/users.py (Diff revision 2)

The issue has been resolved. Show all issues

Maybe worth inversing this?

if i == num_retries - 1:
    raise

logging.error(...)


Maybe getting into the weeds here, but that code actually gets nicer if we change the loop:

for i in range(num_retries + 1):
    ...

    if i == num_retries:
        raise

    ...

Commit:

2d08e972aed0d6934b4a3d4f4b0da95e62b8624a

efe69ac632a9df7f21aed5a3c2adaa9a418bebc2

Diff:

Revision 3 (+117 -82)

Show changes

	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/land.py
	rbtools/commands/login.py
	rbtools/commands/post.py
	rbtools/commands/publish.py
	rbtools/commands/stamp.py
	rbtools/utils/commands.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/utils/users.py (Diff revisions 2 - 3)

The issue has been resolved. Show all issues

There's no need for an else. The raise ends things for this function.

Commit:

efe69ac632a9df7f21aed5a3c2adaa9a418bebc2

7e54390a03bb0bc1c3534515d93da56ec3cd8e3e

Diff:

Revision 4 (+119 -84)

Show changes

	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/land.py
	rbtools/commands/login.py
	rbtools/commands/post.py
	rbtools/commands/publish.py
	rbtools/commands/stamp.py
	rbtools/utils/commands.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-1.0.x (401f6ba)