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: Remove asynchronous chdir callback #3060

Merged
merged 1 commit into from Oct 7, 2019

Conversation

stilley2
Copy link
Contributor

@stilley2 stilley2 commented Oct 1, 2019

Summary

Remove the asynchronous chdir callback in the multiproc plugin

Acknowledgment

  • (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

@stilley2
Copy link
Contributor Author

stilley2 commented Oct 1, 2019

I know this is being discussed in #3047, but I wanted to get CI running on this change to see if it catches anything (my local tests did not).

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #3060 into master will increase coverage by 3.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3060      +/-   ##
==========================================
+ Coverage   64.07%   67.27%   +3.19%     
==========================================
  Files         342      344       +2     
  Lines       44022    44086      +64     
  Branches     5550     5554       +4     
==========================================
+ Hits        28209    29660    +1451     
+ Misses      14664    13603    -1061     
+ Partials     1149      823     -326
Flag Coverage Δ
#smoketests 47.86% <ø> (?)
#unittests 65.17% <ø> (+1.1%) ⬆️
Impacted Files Coverage Δ
nipype/pipeline/plugins/multiproc.py 72.04% <ø> (-0.18%) ⬇️
nipype/interfaces/dynamic_slicer.py 17.47% <0%> (ø) ⬆️
nipype/interfaces/nipy/preprocess.py 45.79% <0%> (ø) ⬆️
nipype/__init__.py 68.08% <0%> (ø)
nipype/testing/__init__.py 88.88% <0%> (ø)
nipype/algorithms/misc.py 45.68% <0%> (+0.11%) ⬆️
nipype/interfaces/afni/preprocess.py 81.97% <0%> (+0.22%) ⬆️
nipype/interfaces/dipy/reconstruction.py 33% <0%> (+0.97%) ⬆️
nipype/interfaces/dipy/tracks.py 31.66% <0%> (+1.11%) ⬆️
nipype/interfaces/dipy/preprocess.py 27.37% <0%> (+1.11%) ⬆️
... and 51 more

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 0fa28d5...47dc288. Read the comment docs.

Copy link
Contributor

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

Thanks for this.

@effigies
Copy link
Member

effigies commented Oct 2, 2019

The problem that led to this, though, was something calling getcwd() from a deleted directory. Has that root cause gone away?

@oesteban
Copy link
Contributor

oesteban commented Oct 2, 2019

It should be. Virtually any getcwd() call from a worker that was initiated in a nonexistent folder would crash. But that problem is addressed by the init hook included in that old PR. The chdir being removed is the result of my desperation at that moment.

@effigies
Copy link
Member

effigies commented Oct 2, 2019

Okay. I'll defer to your memory on that one.

@effigies effigies added this to the 1.3.0 milestone Oct 3, 2019
@effigies
Copy link
Member

effigies commented Oct 7, 2019

@oesteban I was assuming you were going to merge this. Is there a hold-up, or were you waiting for a second approval?

@oesteban oesteban merged commit cffb1f2 into nipy:master Oct 7, 2019
@oesteban
Copy link
Contributor

oesteban commented Oct 7, 2019

Sorry for the delay

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.

None yet

3 participants