Skip to content
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

Merged
merged 6 commits into from
Apr 8, 2018
Merged

Handle exception #78

merged 6 commits into from
Apr 8, 2018

Conversation

shivankar-madaan
Copy link
Contributor

No description provided.

if type(err) == HTTPError:
print ("There has been an HTTP error after three attempts.")
exit (2)
if type(err) == ConnectionError:
Copy link
Member

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:

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)
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments as well
@shivankar-madaan
Copy link
Contributor Author

done

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)
Copy link
Member

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.

Check: https://docs.python.org/3/library/sys.html#sys.exit

Copy link
Member

@KingAkeem KingAkeem left a 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?

@shivankar-madaan
Copy link
Contributor Author

used sys.exit only for the fact,so that the program doesn't end with an exception and there is a clean exit.Can change this if exiting is not right way.

@shivankar-madaan
Copy link
Contributor Author

Are we okay with exit or raising an exception is better?

@KingAkeem
Copy link
Member

Exit is sufficient for now, unless we find a need to raise exceptions.

@PSNAppz
Copy link
Member

PSNAppz commented Apr 2, 2018

@KingAkeem Please review this PR.

Copy link
Member

@KingAkeem KingAkeem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job 👍

@PSNAppz PSNAppz mentioned this pull request Apr 8, 2018
@PSNAppz PSNAppz merged commit bc86db3 into DedSecInside:dev Apr 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants