Skip to content

Test that show failure to close semaphore trackers#676

Merged
ogrisel merged 1 commit intojoblib:masterfrom
fcharras:exception_in_nested_calls_with_loky
Aug 23, 2018
Merged

Test that show failure to close semaphore trackers#676
ogrisel merged 1 commit intojoblib:masterfrom
fcharras:exception_in_nested_calls_with_loky

Conversation

@fcharras
Copy link
Copy Markdown
Contributor

@fcharras fcharras commented May 9, 2018

This test should pass well, but appveyor is going to timeout because some semaphore trackers will not close properly.

@fcharras fcharras force-pushed the exception_in_nested_calls_with_loky branch from f38346c to d1f7039 Compare August 9, 2018 05:16
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 9, 2018

Codecov Report

Merging #676 into master will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
+ Coverage   95.23%   95.35%   +0.11%     
==========================================
  Files          42       42              
  Lines        6128     6134       +6     
==========================================
+ Hits         5836     5849      +13     
+ Misses        292      285       -7
Impacted Files Coverage Δ
joblib/test/test_parallel.py 96.76% <100%> (+0.02%) ⬆️
joblib/_parallel_backends.py 97.2% <0%> (+0.4%) ⬆️
joblib/_store_backends.py 91% <0%> (+0.52%) ⬆️
joblib/backports.py 95.83% <0%> (+2.08%) ⬆️
joblib/disk.py 88.33% <0%> (+6.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec4f1ca...019652a. Read the comment docs.

@fcharras
Copy link
Copy Markdown
Contributor Author

fcharras commented Aug 9, 2018

I've been out of the loop for a while, it seems that this has been fixed ? (except for the sklearn tests that have apparently failed to even start).

Is #588 still worth rebasing ? at the time, this test causing timeout on appveyor was the last blocking point.

Also I've seen Dask has been added as a backend, does it changes anything ?

@fcharras fcharras force-pushed the exception_in_nested_calls_with_loky branch from d1f7039 to b340470 Compare August 9, 2018 20:13
@fcharras fcharras force-pushed the exception_in_nested_calls_with_loky branch from b340470 to 019652a Compare August 9, 2018 20:28
@fcharras
Copy link
Copy Markdown
Contributor Author

fcharras commented Aug 9, 2018

It seems the recent changes on nested parallel instances has fixed this too. It can be closed (or merged as a non regression test ?).

Note: I had to trigger the CI twice because of pipelines failing randomly on travis, before everything finally becomes green.

@tomMoral
Copy link
Copy Markdown
Contributor

This test is more suited to be included in loky.
I tried to do with tommoral/loky#148

ogrisel pushed a commit to joblib/loky that referenced this pull request Aug 23, 2018
@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Aug 23, 2018

I merged the new test upstream in loky. Thanks!

@ogrisel ogrisel closed this Aug 23, 2018
@ogrisel ogrisel reopened this Aug 23, 2018
@ogrisel
Copy link
Copy Markdown
Contributor

ogrisel commented Aug 23, 2018

ACtually let's merge this PR here as well as a non-regression test.

@ogrisel ogrisel merged commit c1b2b50 into joblib:master Aug 23, 2018
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Sep 4, 2018
* tag '0.12.3': (23 commits)
  Release 0.12.3
  Loky 2.2.1 (joblib#760)
  FIX: FileSystemStoreBackend string representation only returning location (path) (joblib#765)
  Add optional dependency on psutil
  MAINT remove brittle time based assertion in test (joblib#761)
  Fix a bug in nesting level computation with FallbackToBackend(SequentialBackend()) (joblib#759)
  Make docstring more consistent with project style
  Improved performance of call_and_shelve (joblib#757)
  Better test name
  Fix default context handling (joblib#754)
  cloudpickle 0.5.5 (joblib#756)
  Fixed error where filter args was consuming kwargs (joblib#750)
  FIX pickle roundtrip for Memory and related classes. (joblib#746)
  test that passes but timeout appveyor because of unclosed semaphore tracker (joblib#676)
  DOC: fine tune compressor example dataset size for ReadTheDocs (joblib#753)
  FIX: MemorizedResult not picklable (joblib#752)
  [MRG] Better message with py27 when lz4 is not installed (joblib#740)
  MNT remove mutable default value for backend_options parameter (joblib#748)
  MNT create test file for _store_backends module (joblib#749)
  DOC consistently use memory rather than mem in memory.rst (joblib#744)
  ...
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Sep 4, 2018
* releases: (23 commits)
  Release 0.12.3
  Loky 2.2.1 (joblib#760)
  FIX: FileSystemStoreBackend string representation only returning location (path) (joblib#765)
  Add optional dependency on psutil
  MAINT remove brittle time based assertion in test (joblib#761)
  Fix a bug in nesting level computation with FallbackToBackend(SequentialBackend()) (joblib#759)
  Make docstring more consistent with project style
  Improved performance of call_and_shelve (joblib#757)
  Better test name
  Fix default context handling (joblib#754)
  cloudpickle 0.5.5 (joblib#756)
  Fixed error where filter args was consuming kwargs (joblib#750)
  FIX pickle roundtrip for Memory and related classes. (joblib#746)
  test that passes but timeout appveyor because of unclosed semaphore tracker (joblib#676)
  DOC: fine tune compressor example dataset size for ReadTheDocs (joblib#753)
  FIX: MemorizedResult not picklable (joblib#752)
  [MRG] Better message with py27 when lz4 is not installed (joblib#740)
  MNT remove mutable default value for backend_options parameter (joblib#748)
  MNT create test file for _store_backends module (joblib#749)
  DOC consistently use memory rather than mem in memory.rst (joblib#744)
  ...
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