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

Changed test file name from .permission_test to UUID to make it thread safe #1110

Merged
merged 3 commits into from
Aug 3, 2023
Merged

Conversation

Sohambutala
Copy link
Contributor

@Sohambutala Sohambutala commented Aug 1, 2023

Thank you @lsetiawan, for this suggestion!

The previous method of verifying necessary permissions in the output folder involved the creation of a temporary file named ".permission_test." However, when dealing with multiple threads in a multithreading scenario, simultaneous attempts by these threads to create, delete, or modify the same file led to a "FileNotFoundError." To address this issue, we've implemented a solution where a unique UUID is now used as the filename instead of a constant name. This way, each file works with its own individual temporary file to verify the required permissions.

pre-commit-ci bot and others added 3 commits August 1, 2023 14:37
updates:
- [github.com/PyCQA/flake8: 6.0.0 → 6.1.0](PyCQA/flake8@6.0.0...6.1.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Aug 1, 2023

Codecov Report

Merging #1110 (c27f96e) into dev (eb92185) will decrease coverage by 0.59%.
Report is 25 commits behind head on dev.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1110      +/-   ##
==========================================
- Coverage   80.71%   80.12%   -0.59%     
==========================================
  Files          67        5      -62     
  Lines        6056      468    -5588     
==========================================
- Hits         4888      375    -4513     
+ Misses       1168       93    -1075     
Flag Coverage Δ
unittests 80.12% <100.00%> (-0.59%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
echopype/utils/io.py 77.19% <100.00%> (-13.29%) ⬇️

... and 65 files with indirect coverage changes

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

@lsetiawan lsetiawan merged commit 4250acf into OSOceanAcoustics:dev Aug 3, 2023
@leewujung
Copy link
Member

Hey @Sohambutala : Thanks for changing this! For the sake of documentation, could you edit the PR comment to put in short explanation on why this change is needed?

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.

4 participants