machinelearning icon indicating copy to clipboard operation
machinelearning copied to clipboard

create unique temporary directories to prevent permission issues

Open ErikApption opened this issue 1 year ago • 2 comments

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.

  • [X] There's a descriptive title that will make sense to other developers some time from now.
  • [X] There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • [X] Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • [X] You have included any necessary tests in the same PR.

ErikApption avatar Jun 09 '24 16:06 ErikApption

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

michaelgsharp avatar Jun 26 '24 16:06 michaelgsharp

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).

ErikApption avatar Jun 26 '24 17:06 ErikApption

/azp run

michaelgsharp avatar Jul 24 '24 19:07 michaelgsharp

Azure Pipelines successfully started running 2 pipeline(s).

azure-pipelines[bot] avatar Jul 24 '24 19:07 azure-pipelines[bot]

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
@@            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 Coverage Δ
Debug 68.83% <100.00%> (+0.16%) :arrow_up:
production 63.08% <100.00%> (+0.13%) :arrow_up:
test 88.99% <ø> (+0.13%) :arrow_up:

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

Files Coverage Δ
src/Microsoft.ML.Core/Data/Repository.cs 79.93% <100.00%> (+0.21%) :arrow_up:

... and 56 files with indirect coverage changes

codecov[bot] avatar Jul 27 '24 01:07 codecov[bot]