Skip to content

Add close() and flush() methods to stream classes for full file-like compatibility#205

Merged
khsrali merged 1 commit into
aiidateam:mainfrom
khsrali:fix_close
Feb 26, 2026
Merged

Add close() and flush() methods to stream classes for full file-like compatibility#205
khsrali merged 1 commit into
aiidateam:mainfrom
khsrali:fix_close

Conversation

@khsrali
Copy link
Copy Markdown
Contributor

@khsrali khsrali commented Feb 26, 2026

These test on aiida-core are flaky

FAILED tests/engine/processes/calcjobs/test_calc_job.py::TestCalcJob::test_get_hash - AttributeError: 'PackedObjectReader' object has no attribute 'close'
FAILED tests/engine/processes/calcjobs/test_calc_job.py::TestCalcJob::test_get_objects_attributes - AttributeError: 'PackedObjectReader' object has no attribute 'close'
FAILED tests/engine/processes/calcjobs/test_calc_job.py::TestCalcJob::test_compute_hash_version_independent - AttributeError: 'PackedObjectReader' object has no attribute 'close'
FAILED tests/engine/processes/calcjobs/test_calc_job.py::test_monitor_version - AttributeError: 'PackedObjectReader' object has no attribute 'close'
FAILED tests/engine/processes/calcjobs/test_calc_job.py::test_monitor_result_override_exit_code - AttributeError: 'PackedObjectReader' object has no attribute 'close'
FAILED tests/engine/processes/calcjobs/test_calc_job.py::test_monitor_result_action_disable_all - AttributeError: 'PackedObjectReader' object has no attribute 'close'
FAILED tests/engine/processes/calcjobs/test_calc_job.py::test_monitor_result_action_disable_self - AttributeError: 'PackedObjectReader' object has no attribute 'close'
FAILED tests/engine/processes/calcjobs/test_calc_job.py::test_restart_after_daemon_reset - AssertionError: The process excepted: AttributeError: 'PackedObjectReader' object has no attribute 'flush'

The flakiness comes down to whether objects are loose or packed in the disk-objectstore at the time the test reads from the repository. The point is disk-objectstore packs objects automatically when certain thresholds are reached.
If the packed object are returned they still should support close() and flush(), becausein aiida-core TextIOWrapper is used as a context manager, its exit calls close(), which propagates to the underlying stream.

The commit adda no-op close() and flush() methods to PackedObjectReader, CallbackStreamWrapper, and ZlibLikeBaseStreamDecompresser.

This was discovered in aiidateam/aiida-core#7206, where async changes results in different tests ordering or something resulting to this issue to pop out.
The methods are intentionally no-ops because these readers don't own the underlying file handles they read from.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.63%. Comparing base (e5cb644) to head (71ae6bc).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #205   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files           8        8           
  Lines        1932     1938    +6     
=======================================
+ Hits         1925     1931    +6     
  Misses          7        7           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@agoscinski
Copy link
Copy Markdown
Contributor

agoscinski commented Feb 26, 2026

I thought about implementing a protocol to enforce this somehow like it was done in #194. But in the end the bug comes from its usage in aiida-core where ignore the type error. I move the consideration of some enforcement through a protocol to PR #194 because I am not really sure what interface we want to enforce.

EDIT: Looking at aiida-core, I think we could solve the issue by just implementing io.BinaryIO Leaving it for PR #194

@khsrali
Copy link
Copy Markdown
Contributor Author

khsrali commented Feb 26, 2026

Ok, agreed with @agoscinski to merge this now and consider the io.BinaryIO idea for #194

@khsrali khsrali merged commit 8caa2ec into aiidateam:main Feb 26, 2026
20 checks passed
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.

2 participants