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

Remove refcounting from enkf_fs #3789

Merged
merged 3 commits into from
Aug 21, 2022
Merged

Conversation

oyvindeide
Copy link
Collaborator

@oyvindeide oyvindeide commented Aug 20, 2022

Issue
All file system creation outside tests are now in python, meaning that automatic refcounting should be handled automatically.

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

enkf_fs is now only owned from python in production code, so refcounting
is handled automatically.
This has the positive side effect of allowing debugging of
the enf_fs constructor as we are no longer trying to read
a property in C that does not exist, this would previously
segfault.
@oyvindeide oyvindeide changed the title Remove refcount Remove refcounting from enkf_fs Aug 20, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #3789 (7567e4b) into main (50bb030) will decrease coverage by 0.02%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##             main    #3789      +/-   ##
==========================================
- Coverage   63.45%   63.43%   -0.03%     
==========================================
  Files         600      600              
  Lines       45433    45372      -61     
  Branches     4098     4096       -2     
==========================================
- Hits        28828    28780      -48     
+ Misses      15345    15334      -11     
+ Partials     1260     1258       -2     
Impacted Files Coverage Δ
src/ert/_c_wrappers/enkf/enkf_main.py 95.98% <ø> (+0.25%) ⬆️
src/ert/libres_facade.py 79.64% <ø> (-0.18%) ⬇️
src/clib/lib/enkf/enkf_fs.cpp 61.84% <50.00%> (-0.06%) ⬇️
src/ert/_c_wrappers/enkf/enkf_fs.py 91.30% <100.00%> (+0.39%) ⬆️
src/ert/_c_wrappers/enkf/enkf_fs_manager.py 83.78% <100.00%> (-0.43%) ⬇️
src/clib/lib/enkf/block_fs_driver.cpp 76.00% <0.00%> (-0.67%) ⬇️
src/ert/data/record/_transformation.py 88.57% <0.00%> (-0.48%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@oyvindeide
Copy link
Collaborator Author

def test_that_delete_works_as_expected(tmpdir):
    with tmpdir.as_cwd():
        assert not (Path("default") / "default.lock").exists()
        fs = EnkfFs.createFileSystem("default")
        assert (Path("default") / "default.lock").exists()
        del fs
        assert not (Path("default") / "default.lock").exists()

not sure if this would add value, but is possible

@oyvindeide oyvindeide merged commit 2308b9b into equinor:main Aug 21, 2022
@oyvindeide oyvindeide deleted the remove_refcount branch August 21, 2022 18:23
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