dwighthubbard / python-dlipower

Python modules to manage Digital Loggers Web Power Switch
Other
54 stars 35 forks source link

Command line tool: intelligent error message if unknown outlet name #8

Closed dtgriscom closed 8 years ago

dtgriscom commented 9 years ago

The command line tool as-is, when presented with an unknown, non-integer outlet name, dumps a stack trace with no recognizable reason why. This change emits an intelligent error message and handles the exception. I'm guessing that this also make the library behavior better. (I'm not a python adept, so I'd be happy for any code suggestions.)

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-1.52%) to 52.11% when pulling 545bcc2cdd50629a0763e2df7a14f106e86968b0 on dtgriscom:master into f8b40f42d012a00c1d68ff63d36b5273355f51a5 on dwighthubbard:master.

dwighthubbard commented 9 years ago

I think it would be better to raise a reasonable exception on these errors so that the calling code can trap the errors if they occur.

Also we should be using a logger for any output outside of functions/methods that are defined as sending output to stdout.

I can show how this is done or your welcome to take a shot at it.

dtgriscom commented 9 years ago

Good points. I expect you'd prefer the dlipower.py methods to throw exceptions, and only the dlipower script to actually output to stdout.

What exception would you like to be thrown if there is no known outlet name? And similarly, what if the argument parses into an int, but is out of range for the given switch?

Thanks, Dan

Daniel T. Griscom, Suitable Systems 1 Centre Street, Suite 204, Wakefield, MA 01880-2400 (781) 665-0053 griscom@suitable.com http://www.suitable.com/

On Jun 1, 2015, at 6:41 PM, Dwight Hubbard notifications@github.com wrote:

I think it would be better to raise a reasonable exception on these errors so that the calling code can trap the errors if they occur.

Also we should be using a logger for any output outside of functions/methods that are defined as sending output to stdout.

I can show how this is done or your welcome to take a shot at it.

— Reply to this email directly or view it on GitHub.

dwighthubbard commented 9 years ago

I'd probably create a new exception named something like InvalidOutlet and have the exception message indicate if it's out of range or an unknown name.

dtgriscom commented 9 years ago

OK, I've switched to exception-mediated errors, with nothing printed to stdout in the module. I pushed it to GitHub, but I'm not sure if I have to open another pull request.

Let me know, Dan

Daniel T. Griscom, Suitable Systems 1 Centre Street, Suite 204, Wakefield, MA 01880-2400 (781) 665-0053 griscom@suitable.com http://www.suitable.com/

On Jun 3, 2015, at 2:54 PM, Dwight Hubbard notifications@github.com wrote:

I'd probably create a new exception named something like InvalidOutlet and have the exception message indicate if it's out of range or an unknown name.

— Reply to this email directly or view it on GitHub.

dtgriscom commented 9 years ago

Have you checked out my updated patch?

Daniel T. Griscom, Suitable Systems 1 Centre Street, Suite 204, Wakefield, MA 01880-2400 (781) 665-0053 griscom@suitable.com http://www.suitable.com/

On Jun 13, 2015, at 1:21 PM, Daniel Griscom griscom@suitable.com wrote:

OK, I've switched to exception-mediated errors, with nothing printed to stdout in the module. I pushed it to GitHub, but I'm not sure if I have to open another pull request.

Let me know, Dan

Daniel T. Griscom, Suitable Systems 1 Centre Street, Suite 204, Wakefield, MA 01880-2400 (781) 665-0053 griscom@suitable.com http://www.suitable.com/

On Jun 3, 2015, at 2:54 PM, Dwight Hubbard notifications@github.com wrote:

I'd probably create a new exception named something like InvalidOutlet and have the exception message indicate if it's out of range or an unknown name.

— Reply to this email directly or view it on GitHub.

dwighthubbard commented 8 years ago

This was manually merged into Master branch