dotnet / machinelearning

ML.NET is an open source and cross-platform machine learning framework for .NET.
https://dot.net/ml
MIT License
9.05k stars 1.88k forks source link

create unique temporary directories to prevent permission issues #7173

Closed ErikApption closed 4 months ago

ErikApption commented 5 months ago

Fixes #7172

This is a tentative fix for #7172 where ml.net fails when multiple users are using same directory. This fix checks if there is already a ml_dotnet if so, it will increase the number until the path is available.

michaelgsharp commented 5 months ago

so Path.GetRandomFileName() should make sure we are getting unique temp directories. What problems are you seeing in regards to this?

ErikApption commented 5 months ago

so Path.GetRandomFileName() should make sure we are getting unique temp directories. What problems are you seeing in regards to this?

Current code has this

string path = Path.Combine(Path.GetFullPath(tempPath), "ml_dotnet", Path.GetRandomFileName());

Issue is the directory name ml_dotnet, not the temp file name. The Path.Combine will create a subdirectory and a random file name inside that directory. The directory name is not unique. When one user processes creates /tmp/ml_dotnet/ for ML.net and a second user runs the same ML.net application at the same time, the second user cannot create /tmp/ml_dotnet/<temp file name> because permissions on /tmp/ml_dotnet/ are restricted to user 1. User 2 doesn't have write permissions (at least on default ubuntu but would be surprised this is different on other distros).

michaelgsharp commented 4 months ago

/azp run

azure-pipelines[bot] commented 4 months ago
Azure Pipelines successfully started running 2 pipeline(s).
codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 68.83%. Comparing base (4bc753a) to head (eb36173). Report is 13 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #7173 +/- ## ========================================== + Coverage 68.66% 68.83% +0.16% ========================================== Files 1262 1267 +5 Lines 257774 259769 +1995 Branches 26660 26946 +286 ========================================== + Hits 176991 178800 +1809 - Misses 73971 74090 +119 - Partials 6812 6879 +67 ``` | [Flag](https://app.codecov.io/gh/dotnet/machinelearning/pull/7173/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | Coverage Δ | | |---|---|---| | [Debug](https://app.codecov.io/gh/dotnet/machinelearning/pull/7173/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `68.83% <100.00%> (+0.16%)` | :arrow_up: | | [production](https://app.codecov.io/gh/dotnet/machinelearning/pull/7173/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `63.08% <100.00%> (+0.13%)` | :arrow_up: | | [test](https://app.codecov.io/gh/dotnet/machinelearning/pull/7173/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `88.99% <ø> (+0.13%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/dotnet/machinelearning/pull/7173?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | Coverage Δ | | |---|---|---| | [src/Microsoft.ML.Core/Data/Repository.cs](https://app.codecov.io/gh/dotnet/machinelearning/pull/7173?src=pr&el=tree&filepath=src%2FMicrosoft.ML.Core%2FData%2FRepository.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#diff-c3JjL01pY3Jvc29mdC5NTC5Db3JlL0RhdGEvUmVwb3NpdG9yeS5jcw==) | `79.93% <100.00%> (+0.21%)` | :arrow_up: | ... and [56 files with indirect coverage changes](https://app.codecov.io/gh/dotnet/machinelearning/pull/7173/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet)