Summary

Remove usages of die() from the API cache

Review Request #7298 — Created May 13, 2015 and submitted May 15, 2015, 2:52 a.m.

Information

Owner

brennie

Repository

RBTools

Branch

release-0.7.x

Bugs

Depends On

Commit

f881b5f...

Reviewers

Groups

rbtools

People

Description

All usages of die in the HTTP cache for the API have now been
replaced to raise a new exception, CacheError, instead.

The --cache-location flag is now accepted by clearcache and the
--disable-cache flag (which was added by mistake) was removed.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
What do you think about moving this into errors.py?	chipx86	May 13, 2015, 10:38 p.m.
Let's build the message in each case, and then do the super() at the end only once.	chipx86	May 13, 2015, 10:38 p.m.
e should be on the next line. Also, the error should be in sentence casing. Same with the others below.	chipx86	May 13, 2015, 10:38 p.m.
local variable 'e' is assigned to but never used	reviewbot	May 13, 2015, 10:42 p.m.
I thought it was okay having this in CacheError. What led to the move?	chipx86	May 14, 2015, 10:27 p.m.

Tool: Pyflakes
Processed Files:
    rbtools/api/cache.py
    rbtools/commands/clearcache.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/cache.py
    rbtools/commands/clearcache.py

rbtools/api/cache.py (Diff revision 1)
The issue has been resolved. Show all issues
```
What do you think about moving this into errors.py?
```

rbtools/api/cache.py (Diff revision 1)

The issue has been resolved. Show all issues

Let's build the message in each case, and then do the super() at the end only once.

rbtools/api/cache.py (Diff revision 1)

The issue has been resolved. Show all issues

e should be on the next line.

Also, the error should be in sentence casing.

Same with the others below.

Change Summary:


Move CacheError into api.errors.
Move CacheError into APICache._die so that CacheError does not reference APICache.
Fix casing.
Build error message only once in _die

Commit:

dcbf9c4f5e91e4dae2027fed194bcad254fe0b71

b0735e102b90b46206fa55398d3ba8f58b4a87e1

Diff:

Revision 2 (+31 -23)

Show changes

	rbtools/api/cache.py
	rbtools/api/errors.py
	rbtools/commands/clearcache.py

Tool: Pyflakes
Processed Files:
    rbtools/api/errors.py
    rbtools/api/cache.py
    rbtools/commands/clearcache.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/errors.py
    rbtools/api/cache.py
    rbtools/commands/clearcache.py

rbtools/api/cache.py (Diff revision 2)
The issue has been resolved. Show all issues
```
 local variable 'e' is assigned to but never used
```

Change Summary:

Fix call to _die

Commit:

b0735e102b90b46206fa55398d3ba8f58b4a87e1

f881b5f1ab5aed9e85dd40855dfb49859d4a3d5a

Diff:

Revision 3 (+31 -23)

Show changes

	rbtools/api/cache.py
	rbtools/api/errors.py
	rbtools/commands/clearcache.py

Tool: Pyflakes
Processed Files:
    rbtools/api/errors.py
    rbtools/api/cache.py
    rbtools/commands/clearcache.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/errors.py
    rbtools/api/cache.py
    rbtools/commands/clearcache.py

rbtools/api/cache.py (Diff revision 3)

The issue has been dropped. Show all issues

I thought it was okay having this in CacheError. What led to the move?

brennie May 14, 2015, 10:27 p.m.

I'm avoiding introducing a circular dependency (because api.cache imports api.errors.CacheError and api.errors would import api.cache.APICache for APICache.DEFAULT_CACHE_PATH).

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (a22856e)