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

BaseEventLoop.run_in_executor shouldn't specify max_workers for default Executor #70983

Closed
hlawrenz mannequin opened this issue Apr 18, 2016 · 8 comments
Closed

BaseEventLoop.run_in_executor shouldn't specify max_workers for default Executor #70983

hlawrenz mannequin opened this issue Apr 18, 2016 · 8 comments
Labels
3.7 (EOL) end of life topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@hlawrenz
Copy link
Mannequin

hlawrenz mannequin commented Apr 18, 2016

BPO 26796
Nosy @gvanrossum, @vstinner, @1st1, @hlawrenz
Files
  • run_in_executor_max_workers.patch
  • run_in_executor_max_workers_vcheck.patch: Patch with version check
  • run_in_executor_max_workers_with_docs.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2016-10-21.21:42:35.572>
    created_at = <Date 2016-04-18.14:43:59.667>
    labels = ['type-bug', '3.7', 'expert-asyncio']
    title = "BaseEventLoop.run_in_executor shouldn't specify max_workers for default Executor"
    updated_at = <Date 2016-10-21.21:42:35.570>
    user = 'https://github.com/hlawrenz'

    bugs.python.org fields:

    activity = <Date 2016-10-21.21:42:35.570>
    actor = 'yselivanov'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-10-21.21:42:35.572>
    closer = 'yselivanov'
    components = ['asyncio']
    creation = <Date 2016-04-18.14:43:59.667>
    creator = 'Hans Lawrenz'
    dependencies = []
    files = ['42506', '42507', '42509']
    hgrepos = []
    issue_num = 26796
    keywords = ['patch']
    message_count = 8.0
    messages = ['263669', '263670', '263671', '263672', '263684', '263685', '279161', '279162']
    nosy_count = 5.0
    nosy_names = ['gvanrossum', 'vstinner', 'python-dev', 'yselivanov', 'Hans Lawrenz']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue26796'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @hlawrenz
    Copy link
    Mannequin Author

    hlawrenz mannequin commented Apr 18, 2016

    In bpo-21527 <http://bugs.python.org/issue21527\> the concurrent.futures.ThreadPoolExecutor was changed to have a default value for max_workers. When asyncio.base_events.BaseEventLoop.run_in_executor creates a default ThreadPoolExecutor it specifies a value of 5 for max_workers, presumably because at the time it was written ThreadPoolExecutor didn't have a default for max_workers. This is confusing because on reading the documentation for ThreadPoolExecutor one might assume that the default specified there is what will be used if a default executor isn't supplied via BaseEventLoop.set_default_executor.

    I propose that BaseEventLoop.run_in_executor be changed to not supply a default for max_workers. If this isn't acceptable, a note ought to be put in the run_in_executor documentation.

    @hlawrenz hlawrenz mannequin added topic-asyncio type-bug An unexpected behavior, bug, or error labels Apr 18, 2016
    @gvanrossum
    Copy link
    Member

    I like this idea for 3.5 and later. I know this is a pain, but I would like the code to have a version check so that the identical code can still be used in Python 3.3 and 3.4. We go through great lengths to keep the asyncio package compatible with those versions so that we can occasionally push a version out to PyPI for 3.3 and 3.4 users. If the versions diverge we would be unable to keep track of other bug fixes.

    @hlawrenz
    Copy link
    Mannequin Author

    hlawrenz mannequin commented Apr 18, 2016

    Thanks, that makes sense. I've attached a patch with a version check.

    @gvanrossum
    Copy link
    Member

    There should at least be a comment at the definition of _MAX_WORKERS that it's only used on Python 3.4 and before.

    Maybe a note in the docs about the default executor would also be useful.

    Otherwise this is exactly what I had in mind -- thanks!

    PS. Could you sign a contributor form online
    https://www.python.org/psf/contrib/contrib-form/

    @hlawrenz
    Copy link
    Mannequin Author

    hlawrenz mannequin commented Apr 18, 2016

    New patch attached. Includes comments and a note in the documentation.

    The documentation note is inside a versionchanged:: 3.5 block. Should this be more specific about the version it changed in? It could be confusing for someone using a version of 3.5 that doesn't have the change.

    Contributor form is signed.

    Thanks!

    @gvanrossum
    Copy link
    Member

    LGTM!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 21, 2016

    New changeset 785597e758a1 by Yury Selivanov in branch '3.5':
    Issue bpo-26796: Don't configure the number of workers for default threadpool executor.
    https://hg.python.org/cpython/rev/785597e758a1

    New changeset 99941cacfc38 by Yury Selivanov in branch '3.6':
    Merge 3.5 (issue bpo-26796)
    https://hg.python.org/cpython/rev/99941cacfc38

    New changeset a475f2e39c6f by Yury Selivanov in branch 'default':
    Merge 3.6 (issue bpo-26796)
    https://hg.python.org/cpython/rev/a475f2e39c6f

    @1st1
    Copy link
    Member

    1st1 commented Oct 21, 2016

    Thank you, Hans!

    @1st1 1st1 added the 3.7 (EOL) end of life label Oct 21, 2016
    @1st1 1st1 closed this as completed Oct 21, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants