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

Add setproctitle module in required #3

Closed
wants to merge 4 commits into from
Closed

Add setproctitle module in required #3

wants to merge 4 commits into from

Conversation

ddurieux
Copy link
Contributor

No description provided.

try:
from setproctitle import setproctitle
except ImportError as err:
setproctitle = lambda s: None
Copy link
Contributor

Choose a reason for hiding this comment

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

I would raise a proper Error exception instead of letting it crash. So that there is no "big" behavior difference

@gst
Copy link
Contributor

gst commented May 19, 2015

This PR should only have +setproctitle in the requirements file then ;)

The others "blank" changes (space / TAB) should be in a separate PR if you want to clean that kind of spaces / tabs ..

otherwise +1 ;)

Also : yes, I think we can keep the try: import setproctitle except .. for now if not for always. It can effectively be better to keep it in case something is wrong with the installation of this library and that it can't be imported.
But : you could/should add a warnings.warn(..) in the except clause to have an eventual trace of the actual import error.

@gst
Copy link
Contributor

gst commented May 27, 2015

@ddurieux : ping ?

@Seb-Solon
Copy link
Contributor

Commit have to be squashed IMO

@ddurieux ddurieux changed the title Add setproctitle module in required [wip] Add setproctitle module in required May 28, 2015
@gst
Copy link
Contributor

gst commented May 28, 2015

yep, squash that, rebase it on develop, and it'll be merged ;)

@ddurieux ddurieux changed the title [wip] Add setproctitle module in required Add setproctitle module in required Jun 9, 2015
@gst
Copy link
Contributor

gst commented Jun 9, 2015

this can be closed per #67

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants