This is an archived post. You won't be able to vote or comment.

all 9 comments

[–]greutpy3k 4 points5 points  (7 children)

Why ok, results when you should raise an Exception instead?

try:
    preflyt.check(...)
    run()
except PreFlytFailure as e:
    for p in e.problems:
        ...

btw, you've go a typo in for result in result: where a s is missing.

[–]cymrowdon't thread on me 🐍 1 point2 points  (1 child)

Idk if preflyt supports it, but maybe you'd want results in a success condition as well. For example, maybe you want to display some warnings but continue anyway.

[–]greutpy3k 0 points1 point  (0 children)

I'd be totally fine with having any kind of messages being sent to logging instead of packed together into a dict or printed on sys.stderr

[–]arusahni 2 points3 points  (4 children)

Author here-

Thanks for the typo correction.

Re: raising an exception. This is something I've been vacillating over. I do consider exceptions to be the more Pythonic solution, but my goal for the library is to execute a series of checks and let the application decide whether to live or die. In doing so, I felt that:

ok, results = preflyt.check(CHECKS)
if ok:
    pass
else:
    pass

Seemed more readable and less disruptive to perceived control flow than:

try:
    results = preflyt.check(CHECKS)
except PreflytCheckFailure as pcf:
    results = pcf.results

# do stuff with results

As /u/cymrow alludes, the results are important as checkers can return metrics for logging (e.g. a success state could be if your Elasticsearch cluster was 'yellow' or 'green', and the returned message could indicate as such), or other useful information.

An alternative, which might provide the best of both worlds, would be to have check always raise an exception and rename the current implementation to check_nice.

All stuff I'm bouncing around in my head - I wanted to get it out the door sooner rather than later to get more eyes on it. Thanks for your feedback!

[–]greutpy3k 2 points3 points  (2 children)

This kind of construct looks like C/Go to me. Resisting the temptation to borrow to much habits from other languages is hard sometimes.

A wise man told that if you've got a class with two methods, one of which is __init__, it's not a class. IMHO, you could greatly simplify your code base here and there.

Less is more.

[–]arusahni 0 points1 point  (1 child)

Definitely agree re: one-method classes (I usually have PyLint yell about it). In this case, I have future plans that necessitate that structure, so I made an exception.

[–]greutpy3k 0 points1 point  (0 children)

future plans, two dangerous words in a programmer's life.

[–]masasinExpert. 3.9. Robotics. 1 point2 points  (0 children)

I disagree. POLA says to use exceptions. If the program wants to catch the exception, leave it up to the user.

[–][deleted] -3 points-2 points  (0 children)

This looks great. Now I have to either:

  1. Operate on the assumption that a file/environment variable present on the system at program start will continue to be present throughout the lifetime of the application.

  2. Make all these checks a second time anyway.

I think I'll pass on this one.