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

Replace use of Thread with ThreadPoolExecutor #391

Closed
wants to merge 3 commits into from

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Oct 9, 2019

Also log any error while evaluating callbacks, not just if thread creation fails.

@rotu
Copy link
Contributor Author

rotu commented Oct 9, 2019

Hmm... These CI failures don't look like my fault:
EDIT: it seems to be a problem with pytest==4.2.0

Installing collected packages: attrs, colorama, more-itertools, zipp, importlib-metadata, pluggy, py, atomicwrites, six, pytest, pythonnet, cefpython3
Successfully installed atomicwrites-1.3.0 attrs-19.2.0 cefpython3-66.0 colorama-0.4.1 importlib-metadata-0.23 more-itertools-7.2.0 pluggy-0.13.0 py-1.8.0 pytest-4.2.0 pythonnet-2.4.0 six-1.12.0 zipp-0.6.0
You are using pip version 18.1, however version 19.2.3 is available.
You should consider upgrading via the 'python -m pip install --upgrade pip' command.
%CMD_IN_ENV% python -m pytest tests/test_bg_color.py -s
Traceback (most recent call last):
  File "C:\Python36-x64\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python36-x64\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Python36-x64\lib\site-packages\pytest.py", line 89, in <module>
    raise SystemExit(pytest.main())
  File "C:\Python36-x64\lib\site-packages\_pytest\config\__init__.py", line 61, in main
    config = _prepareconfig(args, plugins)
  File "C:\Python36-x64\lib\site-packages\_pytest\config\__init__.py", line 182, in _prepareconfig
    config = get_config()
  File "C:\Python36-x64\lib\site-packages\_pytest\config\__init__.py", line 156, in get_config
    pluginmanager.import_plugin(spec)
  File "C:\Python36-x64\lib\site-packages\_pytest\config\__init__.py", line 530, in import_plugin
    __import__(importspec)
  File "C:\Python36-x64\lib\site-packages\_pytest\tmpdir.py", line 25, in <module>
    class TempPathFactory(object):
  File "C:\Python36-x64\lib\site-packages\_pytest\tmpdir.py", line 35, in TempPathFactory
    lambda p: Path(os.path.abspath(six.text_type(p)))
TypeError: attrib() got an unexpected keyword argument 'convert'

@rotu rotu force-pushed the thread_pool branch 3 times, most recently from 4797ed0 to dc6e737 Compare October 9, 2019 21:29
Also log any error while evaluating callbacks, not just if thread creation fails.
@rotu
Copy link
Contributor Author

rotu commented Oct 9, 2019

ThreadPoolExecutor doesn't exist in Python2, so if legacy Python support is necessary, please close this PR unmerged. nevermind. using https://pypi.org/project/futures/

@rotu rotu closed this Oct 9, 2019
@rotu rotu deleted the thread_pool branch October 9, 2019 21:41
@rotu rotu restored the thread_pool branch October 9, 2019 21:41
@rotu
Copy link
Contributor Author

rotu commented Oct 9, 2019

did not intend to close this

@rotu rotu reopened this Oct 9, 2019
@r0x0r
Copy link
Owner

r0x0r commented Oct 10, 2019

Is this a fix to #390? I have tested with your example and it hangs randomly.

@rotu
Copy link
Contributor Author

rotu commented Oct 10, 2019

No, it’s not. This PR is intended to only reduce the maximum number of threads and to increase logging coverage when a python callback errors out.

@r0x0r
Copy link
Owner

r0x0r commented Oct 11, 2019

Got it. This is a welcomed change.
As I mentioned in the previous comment this change hangs the app with a for loop.

    for (let i = 0; i < 10; i++) {
        console.log('request ' + i);
        api.echo(i).then((j) => {
            console.log('response ' + i)
        })
    }
class JSApi:
    def echo(self, value):
        print(value)
        return value

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale label Nov 11, 2019
@github-actions github-actions bot closed this Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants