Repository: wandb/wandb
Pull request: fix(sdk): fix context manager usage in disabled mode
Description
-----------
<!--
Include reference to internal ticket "Fixes WB-NNNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
-->
Fixes #8112.

<!--
NEW: We're using a new changelog format that's more useful for users. Please
see CHANGELOG.md for details and update on relevant changes such as feature
additions, bug fixes, or removals/deprecations.
-->
- [x] I updated CHANGELOG.md, or it's not applicable


Testing
-------
How was this PR tested?

<!--
Ensure PR title compliance with the [conventional commits standards](https://github.com/wandb/wandb/blob/main/CONTRIBUTING.md#conventional-commits)
-->


Problem statement:
[CLI]: initializing twice leads to `AttributeError: 'Run' object has no attribute '_telemetry_obj'.`
### Describe the bug

<!--- Description of the issue below  -->
## Description

Starting from `wandb==0.17.6`, I noticed an issue that if I initialize twice as `run = wandb.init(mode="disabled")` I get `AttributeError: 'Run' object has no attribute '_telemetry_obj'`.

Ordinary mode, i.e., `run = wandb.init()` can be called many times.

Up until 0.17.5, `run = wandb.init(mode="disabled")` returned an instance of `RunDisabled`. This seems to have changed from 0.17.6.

<!--- A minimal code snippet between the quotes below  -->
## Steps to reproduce the issue
```python
import wandb
run = wandb.init(mode="disabled")
run = wandb.init(mode="disabled")
```

<!--- A full traceback of the exception in the quotes below -->
## Traceback
```shell
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ryoto/conda/envs/materials/lib/python3.10/site-packages/wandb/sdk/wandb_init.py", line 1191, in init
    wandb._sentry.reraise(e)
  File "/home/ryoto/conda/envs/materials/lib/python3.10/site-packages/wandb/analytics/sentry.py", line 155, in reraise
    raise exc.with_traceback(sys.exc_info()[2])
  File "/home/ryoto/conda/envs/materials/lib/python3.10/site-packages/wandb/sdk/wandb_init.py", line 1176, in init
    wi.setup(kwargs)
  File "/home/ryoto/conda/envs/materials/lib/python3.10/site-packages/wandb/sdk/wandb_init.py", line 211, in setup
    with telemetry.context(obj=self._init_telemetry_obj) as tel:
  File "/home/ryoto/conda/envs/materials/lib/python3.10/site-packages/wandb/sdk/lib/telemetry.py", line 43, in __exit__
    self._run._telemetry_callback(self._obj)
  File "/home/ryoto/conda/envs/materials/lib/python3.10/site-packages/wandb/sdk/wandb_run.py", line 799, in _telemetry_callback
    self._telemetry_obj.MergeFrom(telem_obj)
AttributeError: 'Run' object has no attribute '_telemetry_obj'. Did you mean: '_telemetry_flush'?
```


### Additional Files

_No response_

### Environment

WandB version: 0.17.6
OS: Ubuntu 22.04

Python version: 3.10

Versions of relevant libraries:


### Additional Context

_No response_

Hints from issue discussion:
I'm also hitting this :(
Looks like this is already fixed in #8101, but not yet released.
Setting `wandb.init(mode="disabled")` seems to also break the context manager usage like:
```
with wandb.init() as run:
  ... do training ...
```
with the same error as reported above.
I am also hitting this bug. I think it's quite critical as it prevents me from debugging my runs!
WandB Internal User commented: 
itssimon commented: 
I'm also hitting this :(

WandB Internal User commented: 
itssimon commented: 
Looks like this is already fixed in #8101, but not yet released.

Jason Davenport commented: 
Hello there,

Thanks for bringing this to our attention. I'm glad to let you know that our team is already aware of this issue and is actively working on a solution. We appreciate your patience and understanding.

We'll make sure to keep you in the loop with any updates or progress on this matter. If you have any additional questions or concerns in the meantime, please don't hesitate to reach out.

Thanks again for your report and for being a part of our community!

Best regards,
Jason

BTW: Is there any rational behind the weird API funneling everything through wand.init and having static global variables all over the place? Seems like a super smelly design decision :D.
can ok？use 0.17.7？
> can ok？use 0.17.7？

test ok！

Commit message: add a test

Patch to review:
diff --git a/tests/unit_tests/test_mode_disabled.py b/tests/unit_tests/test_mode_disabled.py
index 683bce498de..794f07f3d8b 100644
--- a/tests/unit_tests/test_mode_disabled.py
+++ b/tests/unit_tests/test_mode_disabled.py
@@ -35,3 +35,19 @@ def test_disabled_can_pickle():
 
     with tempfile.NamedTemporaryFile() as temp_file:
         pickle.dump(run, temp_file)
+
+
+def test_disabled_context_manager():
+    with wandb.init(mode="disabled") as run:
+        run.log({"a": 1})
+        run.summary.update({"b": 2})
+        run.config.update({"c": 3})
+        run.log_artifact("artifact")
+        run.use_artifact("artifact")
+        run.log_model("model")
+        run.use_model("model")
+        run.link_model("model")
+        run.define_metric("metric")
+        run.mark_preempting()
+        run.plot_table("table")
+        run.alert("alert")
diff --git a/wandb/sdk/wandb_run.py b/wandb/sdk/wandb_run.py
index 44ddb1268fa..b22d3f09846 100644
--- a/wandb/sdk/wandb_run.py
+++ b/wandb/sdk/wandb_run.py
@@ -2160,6 +2160,9 @@ def _finish(
         quiet: Optional[bool] = None,
     ) -> None:
         logger.info(f"finishing run {self._get_path()}")
+        if self.disabled:
+            return
+
         with telemetry.context(run=self) as tel:
             tel.feature.finish = True
 


Code review comments:
diff_hunk: @@ -2160,6 +2160,9 @@ def _finish(
         quiet: Optional[bool] = None,
     ) -> None:
         logger.info(f"finishing run {self._get_path()}")
+        if self.disabled:
+            return
path: wandb/sdk/wandb_run.py
text: @user1:
why not just update the definition in `_make_run_disabled`? i feel like we need to clean this code path a lot, now this disable is scattered all over and the source of our issues :(