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

fix PYTHONASYNCIODEBUG and AIOHTTP_NO_EXTENSIONS usage #551

Merged
merged 2 commits into from
Oct 6, 2015

Conversation

rutsky
Copy link
Member

@rutsky rutsky commented Oct 4, 2015

PYTHONASYNCIODEBUG environment variable enables asyncio debug mode if it's any non-empty string (docs).

In code it's actually implemented in the following way, which corresponds to the documented behaviour:

self._debug = (not sys.flags.ignore_environment
                       and bool(os.environ.get('PYTHONASYNCIODEBUG')))

Not documented AIOHTTP_NO_EXTENSIONS environment variable used in the same way, so it's behaviour is the same.

BTW, maybe AIOHTTP_NO_EXTENSIONS should be documented?

PYTHONASYNCIODEBUG environment variable enables asyncio debug mode
if it's any **non-empty** string
(see <https://docs.python.org/3/using/cmdline.html#envvar-PYTHONASYNCIODEBUG>).

In code it's actually implemented in the following way, which corresponds
to documented behaviour:

    self._debug = (not sys.flags.ignore_environment
                   and bool(os.environ.get('PYTHONASYNCIODEBUG')))

Not documented `AIOHTTP_NO_EXTENSIONS` environment variable used in the same
way, so it's behaviour is the same.
- PYTHONASYNCIODEBUG=0 AIOHTTP_NO_EXTENSIONS=0
- PYTHONASYNCIODEBUG=1 AIOHTTP_NO_EXTENSIONS=1
- PYTHONASYNCIODEBUG=0 AIOHTTP_NO_EXTENSIONS=1
- PYTHONASYNCIODEBUG=nonemptystring AIOHTTP_NO_EXTENSIONS=
Copy link
Member

Choose a reason for hiding this comment

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

I believe one character should be enough

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I prefer to explicitly state meaning of the environment variable to prevent confusions such as fixed in this PR.

Setting PYTHONASYNCIODEBUG to yes, Y, 1, True, T etc, most probably will make reader to think that to negate meaning of the variable he should use no, N, 0, False, F, etc value.

Maybe use shorter value, e.g. set instead of nonemptystring?

Copy link
Member

Choose a reason for hiding this comment

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

It affects aiohttp contributors only.
Believe me I'll never again forget the rule but I don't want to see nonemptystring every time when I open .travis.yml.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this affects only aiohttp contributors. Since this is Open Source project it's common to use any part of it in other projects. For example I used this .travis.yml as reference in aiohttp_cors, and most probably will use again in other asyncio-based project.

What about single character + comment?

@rutsky rutsky force-pushed the fix_PYTHONASYNCIODEBUG_usage branch from 1e16a4c to e28ca41 Compare October 5, 2015 19:57
asvetlov added a commit that referenced this pull request Oct 6, 2015
fix PYTHONASYNCIODEBUG and AIOHTTP_NO_EXTENSIONS usage
@asvetlov asvetlov merged commit 55b7cb0 into aio-libs:master Oct 6, 2015
@asvetlov
Copy link
Member

asvetlov commented Oct 6, 2015

Thanks!

rutsky added a commit to rutsky/aiohttp_cors that referenced this pull request Nov 1, 2015
@lock
Copy link

lock bot commented Oct 30, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants