-
-
Notifications
You must be signed in to change notification settings - Fork 555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle exception #78
Handle exception #78
Conversation
modules/pagereader.py
Outdated
if type(err) == HTTPError: | ||
print ("There has been an HTTP error after three attempts.") | ||
exit (2) | ||
if type(err) == ConnectionError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above comment if err is ConnectionError:
modules/pagereader.py
Outdated
raise("There has been a connection error after three attempts.") | ||
if type(err) == HTTPError: | ||
print ("There has been an HTTP error after three attempts.") | ||
exit (2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment explaining what the exit code 2 does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed exit code to 1
as per
https://docs.python.org/3/library/sys.html#sys.exit
Added comments as well
done |
modules/pagereader.py
Outdated
if type(err) == HTTPError: | ||
print ("There has been an HTTP error after three attempts.") | ||
#exit 2 to indicate that there has been exception | ||
exit (2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant could you explain what the error code indicates. For example, an error code of 2 in Unix systems usually symbolizes a command line syntax error. We should probably use an error code of 1 because that is used for most other applications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks fine now, I only have one question. Why did you decide to switch to the sys.exit
module instead of just raising an exception?
used |
Are we okay with exit or raising an exception is better? |
Exit is sufficient for now, unless we find a need to raise exceptions. |
@KingAkeem Please review this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job 👍
No description provided.