wmayner / pyphi

A toolbox for integrated information theory.
https://journals.plos.org/ploscompbiol/article?id=10.1371/journal.pcbi.1006343
Other
368 stars 98 forks source link

atomic_write_yaml() fails to move fresh pyphi_config.yml ? #60

Closed isacdaavid closed 2 years ago

isacdaavid commented 2 years ago

I encountered this new thing while trying to run the test suite against the iit-4.0 branch, while also excluding the tests I had already found to be anti-sufficient for passing the suite.

It fails to collect test/test_subsystem_phi_max.py before actual testing starts.

py.test --slow -k 'not test foo and not test bar and not test blablabla'

========================================= test session starts ==========================================
platform linux -- Python 3.10.5, pytest-7.1.2, pluggy-1.0.0 -- /usr/bin/python
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/isaac/pyphi/.hypothesis/examples')
rootdir: /home/isaac/pyphi, configfile: pytest.ini, testpaths: pyphi, test, docs
plugins: hypothesis-6.47.2, lazy-fixture-0.6.3, anyio-3.6.1
collected 641 items / 1 error                                                                          

=======================ERRORS =========================
_________ERROR collecting test/test_subsystem_phi_max.py ________

test/test_subsystem_phi_max.py:16: in <module>
    with config.override(REPERTOIRE_DISTANCE="EMD"):
pyphi/conf.py:367: in __enter__
    self.conf.load_dict(self.new_values)
pyphi/conf.py:314: in load_dict
    setattr(self, k, v)
pyphi/conf.py:293: in __setattr__
    super().__setattr__(name, value)
pyphi/conf.py:220: in __set__
    self._callback(obj)
pyphi/conf.py:246: in _callback
    self.on_change(obj)
pyphi/conf.py:275: in wrapper
    func(*args, **kwargs)
pyphi/conf.py:1061: in on_change_global
    atomic_write_yaml(config.snapshot(), PYPHI_MANAGED_CONFIG_PATH)
pyphi/conf.py:1051: in atomic_write_yaml
    Path(f.name).rename(path)
/usr/lib/python3.10/pathlib.py:1234: in rename
    self._accessor.rename(self, target)
E   OSError: [Errno 18] Invalid cross-device link: '/tmp/tmp7jw9h8j6' -> '__pyphi_cache__/config/pyphi_config.yml'
======================================== Hypothesis Statistics =========================================
======================================= short test summary info ========================================
ERROR test/test_subsystem_phi_max.py - OSError: [Errno 18] Invalid cross-device link: '/tmp/tmp7jw9h8...
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=========================================== 1 error in 0.51s ===========================================

According to the internet, os.rename() can't move files across filesystems, which is the case in my computer (although that doesn't explain why I'm only seeing the error until now):

$ df -h /tmp

Filesystem      Size  Used Avail Use% Mounted on
tmpfs            16G  1.5M   16G   1% /tmp

People suggest doing it in 2 steps: copying, removing. I simply bypassed this by making atomic_write_yaml() write directly to the final path, but I don't know if either solution is "atomic" enough.

isacdaavid commented 2 years ago

"although that doesn't explain why I'm only seeing the error until now"

Now I see the function was moved from utils.py to conf.py

wmayner commented 2 years ago

Indeed writing directly to the file is not atomic enough. It may not be complete when another process tries to read from it, and if other processes try to write to it concurrently, there can be contention for the file lock. I can look into other options.