Skip to content

create unique temporary directories to prevent permission issues - #7173

Merged
michaelgsharp merged 2 commits into
dotnet:mainfrom
ErikApption:main
Jul 27, 2024
Merged

michaelgsharp merged 2 commits into
dotnet:mainfrom
ErikApption:main

Conversation

@ErikApption

@ErikApption ErikApption commented Jun 9, 2024 •

Copy link
Copy Markdown
Contributor

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.

  • There's a descriptive title that will make sense to other developers some time from now.
  • 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.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Pull requests submitted by community contributors label Jun 9, 2024
@michaelgsharp

Copy link
Copy Markdown
Contributor

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

@ErikApption

ErikApption commented Jun 26, 2024 •

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@codecov

codecov Bot commented Jul 27, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

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%) ⬆️
production 63.08% <100.00%> (+0.13%) ⬆️
test 88.99% <ø> (+0.13%) ⬆️

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%) ⬆️

... and 56 files with indirect coverage changes

@michaelgsharp
michaelgsharp merged commit b4a4b10 into dotnet:main Jul 27, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community-contribution Pull requests submitted by community contributors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Directory permission exception with multiple concurrent users on linux

2 participants