chevah / compat

Chevah OS Compatibility Layer
Other
4 stars 1 forks source link

[#682] Don't lock the file when open for reading. #683

Closed adiroiban closed 1 year ago

adiroiban commented 1 year ago

Scope

Fixes #682

This tries to get the same behaviour over linux and Windows when opening a file for reading.

The default Python code locks the file on Windows, but the file is not locked on Linux.

Changes

Implement custom Windows open code.

As a drive-by, it updates GitHub Actions to latest version as the old version will be deprecated.

How to try and test the changes

reviewers: @danuker

Check that changes make sense

There is a chevah/server PR in which this code is used https://github.com/chevah/server/pull/5998

codecov[bot] commented 1 year ago

Codecov Report

Merging #683 (46607e3) into master (e2ff275) will increase coverage by 0.31%. The diff coverage is 95.55%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #683 +/- ## ========================================== + Coverage 76.61% 76.92% +0.31% ========================================== Files 58 58 Lines 8059 8179 +120 Branches 667 668 +1 ========================================== + Hits 6174 6292 +118 - Misses 1734 1736 +2 Partials 151 151 ``` | [Impacted Files](https://codecov.io/gh/chevah/compat/pull/683?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chevah) | Coverage Δ | | |---|---|---| | [chevah/compat/nt\_filesystem.py](https://codecov.io/gh/chevah/compat/pull/683/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chevah#diff-Y2hldmFoL2NvbXBhdC9udF9maWxlc3lzdGVtLnB5) | `79.83% <91.42%> (+1.28%)` | :arrow_up: | | [chevah/compat/tests/normal/test\_filesystem.py](https://codecov.io/gh/chevah/compat/pull/683/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chevah#diff-Y2hldmFoL2NvbXBhdC90ZXN0cy9ub3JtYWwvdGVzdF9maWxlc3lzdGVtLnB5) | `98.37% <97.00%> (+0.25%)` | :arrow_up: | | [chevah/compat/testing/testcase.py](https://codecov.io/gh/chevah/compat/pull/683/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chevah#diff-Y2hldmFoL2NvbXBhdC90ZXN0aW5nL3Rlc3RjYXNlLnB5) | `76.92% <0.00%> (-0.52%)` | :arrow_down: | | [chevah/compat/posix\_filesystem.py](https://codecov.io/gh/chevah/compat/pull/683/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chevah#diff-Y2hldmFoL2NvbXBhdC9wb3NpeF9maWxlc3lzdGVtLnB5) | `93.05% <0.00%> (-0.43%)` | :arrow_down: |
adiroiban commented 1 year ago

This is ready for a review.

No need to run manual tests.

We have the automated tests that are also executed on Windows

needs-review

adiroiban commented 1 year ago

Many thanks for another great review.

I don't know why coverage is not happy, but we can deal with it later.

I hope I have address all the major issues

needs-review

adiroiban commented 1 year ago

Thanks for the review.

There were a few changes and updated tests.

The failing tests were observed on chevah/server

I have implemented more tests into chevah/compat. I hope this should give us a better coverage for the intended behaviour

needs-review

adiroiban commented 1 year ago

Thanks. I have merged this without a new-release.

The 0.65.5 is already used in chevah/server for a long time