Repository: wandb/wandb
Pull request: fix(artifacts): more informative message when failing to create staging artifact directory 
Fixes #5055

Description
-----------
Catch `PermissionsError` when creating the staging directory so we can raise a more informative error message with a suggested fix.

Testing
-------
1 new test.

Checklist
-------
- [x] Include reference to internal ticket "Fixes WB-NNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
- [x] Ensure PR title compliance with the [conventional commits standards](https://github.com/wandb/wandb/blob/main/CONTRIBUTING.md#conventional-commits)


Problem statement:
[Q] Change staging dir
Currently when the run ends I am getting this error trace. I think it's because I do not have write access to where the staging directory is selected by default. How do I change that?

```python
Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace.
wandb: Waiting for W&B process to finish... (failed 1). Press Control-C to abort syncing.
Thread SenderThread:
Traceback (most recent call last):
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/internal_util.py", line 49, in run
    self._run()
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/internal_util.py", line 100, in _run
    self._process(record)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/internal.py", line 329, in _process
    self._sm.send(record)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/sender.py", line 343, in send
    send_handler(record)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/sender.py", line 365, in send_request
    send_handler(record)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/sender.py", line 615, in send_request_defer
    self._flush_job()
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/sender.py", line 1567, in _flush_job
    artifact = self._job_builder.build()
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/internal/job_builder.py", line 214, in build
    f.write(json.dumps(source_info, indent=4))
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/contextlib.py", line 120, in __exit__
    next(self.gen)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/wandb_artifacts.py", line 412, in new_file
    self.add_file(path, name=name)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/wandb_artifacts.py", line 433, in add_file
    return self._add_local_file(name, local_path, digest=digest)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/wandb_artifacts.py", line 749, in _add_local_file
    with get_new_staging_file() as f:
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/interface/artifacts.py", line 999, in get_new_staging_file
    return tempfile.NamedTemporaryFile(dir=get_staging_dir(), delete=False)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/interface/artifacts.py", line 994, in get_staging_dir
    filesystem.mkdir_exists_ok(path)
  File "/arc/project/st-jiaruid-1/miniconda3/envs/mrl-2/lib/python3.8/site-packages/wandb/sdk/lib/filesystem.py", line 18, in mkdir_exists_ok
    raise PermissionError(f"{dir_name!s} is not writable")
PermissionError: /home/shenoy/.local/share/wandb/artifacts/staging is not writable
wandb: ERROR Internal wandb error: file data was not synced
```

Hints from issue discussion:
I think I found the issue, I need to change the `env` variable `WANDB_DATA_DIR`.
Thank you @shenoynikhil for letting us know the solution to your issue. I will be closing it now. If you have any other issue in the future please feel free to contact us!
@shenoynikhil thanks for reporting! @billmdevs it seems like we can do a little better reporting on this error message; I'm submitting https://github.com/wandb/wandb/pull/5067 to prevent this from being confusing in the future.

Commit message: w/e do it the dumb way

Patch to review:
diff --git a/tests/pytest_tests/unit_tests/test_artifacts/test_storage.py b/tests/pytest_tests/unit_tests/test_artifacts/test_storage.py
index ec012a54b2a..f305bf8afaf 100644
--- a/tests/pytest_tests/unit_tests/test_artifacts/test_storage.py
+++ b/tests/pytest_tests/unit_tests/test_artifacts/test_storage.py
@@ -6,6 +6,7 @@
 import pytest
 import wandb
 from wandb.sdk import wandb_artifacts
+from wandb.sdk.interface import artifacts
 
 
 def test_opener_rejects_append_mode(cache):
@@ -345,3 +346,17 @@ class UnfinishedStorageHandler(wandb_artifacts.StorageHandler):
         ush.load_path(manifest_entry=None)
     with pytest.raises(NotImplementedError):
         ush.store_path(artifact=None, path="")
+
+
+def test_unwritable_staging_dir(monkeypatch):
+    # Use a non-writable directory as the staging directory.
+    # CI just doesn't care about permissions, so we're patching os.makedirs 🙃
+    def nope(*args, **kwargs):
+        raise OSError(13, "Permission denied")
+
+    monkeypatch.setattr(os, "makedirs", nope)
+
+    with pytest.raises(PermissionError) as excinfo:
+        _ = artifacts.get_new_staging_file()
+
+    assert "Set WANDB_DATA_DIR to " in str(excinfo.value)
diff --git a/wandb/sdk/interface/artifacts.py b/wandb/sdk/interface/artifacts.py
index de4d34ff83d..0697e1b7710 100644
--- a/wandb/sdk/interface/artifacts.py
+++ b/wandb/sdk/interface/artifacts.py
@@ -986,7 +986,14 @@ def get_artifacts_cache() -> ArtifactsCache:
 
 def get_staging_dir() -> FilePathStr:
     path = os.path.join(env.get_data_dir(), "artifacts", "staging")
-    mkdir_exists_ok(path)
+    try:
+        mkdir_exists_ok(path)
+    except OSError as e:
+        raise PermissionError(
+            f"Unable to write staging files to {path}. "
+            "Set WANDB_DATA_DIR to a directory you have write access to."
+        ) from e
+
     return FilePathStr(os.path.abspath(os.path.expanduser(path)))
 
 


Code review comments:
diff_hunk: @@ -986,7 +986,14 @@ def get_artifacts_cache() -> ArtifactsCache:
 
 def get_staging_dir() -> FilePathStr:
     path = os.path.join(env.get_data_dir(), "artifacts", "staging")
-    mkdir_exists_ok(path)
+    try:
+        mkdir_exists_ok(path)
+    except OSError as e:
+        raise PermissionError(
+            f"Unable to write staging files to {path}. "
+            "Set WANDB_DATA_DIR to a directory you have write access to."
path: wandb/sdk/interface/artifacts.py
text: @user1:
maybe we can use the `wandb.env` variable instead of hard-coding the name?

what do you think about this small re-phrasing?
```
Cannot write staging files to {path} due to lack of write permission. 

To fix this problem, please set  {env.DATA_DIR}  to a directory 

where you have the necessary write access.
```

diff_hunk: @@ -345,3 +346,17 @@ class UnfinishedStorageHandler(wandb_artifacts.StorageHandler):
         ush.load_path(manifest_entry=None)
     with pytest.raises(NotImplementedError):
         ush.store_path(artifact=None, path="")
+
+
+def test_unwritable_staging_dir(monkeypatch):
+    # Use a non-writable directory as the staging directory.
+    # CI just doesn't care about permissions, so we're patching os.makedirs 🙃
+    def nope(*args, **kwargs):
+        raise OSError(13, "Permission denied")
+
+    monkeypatch.setattr(os, "makedirs", nope)
+
+    with pytest.raises(PermissionError) as excinfo:
path: tests/pytest_tests/unit_tests/test_artifacts/test_storage.py
text: @user1:
you can use match and then you don't need to write the assertion (obviously stylistic choice)

@author:
I actually forgot that `pytest.raises` accepted a match parameter!