Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add better error handling when tearing down service #4161

Merged
merged 13 commits into from Aug 26, 2022
Merged

Conversation

kptkin
Copy link
Contributor

@kptkin kptkin commented Aug 22, 2022

Fixes WB-10725

Description

What does the PR do?

When using wandb.teardown the system could be in a bad state. For example in a notebook setup where the service was closed, due to some kill signal, but token env variable is still present and the setup singleton was not removed. This PR adds better error handling for cases when some of the teardown operations could cause an exception and guarantee that the env variable is cleared to force reconnect on a new call to init.

Testing

How was this PR tested?

Add a unit tests that tries to create a bad state and see that the error was handled.

Checklist

  • Include reference to internal ticket "Fixes WB-NNNN" (and github issue "Fixes #NNNN" if applicable)

@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #4161 (5912d81) into master (43e8ead) will increase coverage by 0.00%.
The diff coverage is 90.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4161   +/-   ##
=======================================
  Coverage   82.65%   82.66%           
=======================================
  Files         256      256           
  Lines       32541    32546    +5     
=======================================
+ Hits        26896    26903    +7     
+ Misses       5645     5643    -2     
Flag Coverage Δ
functest 55.96% <70.00%> (+<0.01%) ⬆️
unittest 73.38% <90.00%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
wandb/sdk/wandb_manager.py 94.07% <90.00%> (+0.22%) ⬆️
wandb/sdk/lib/sock_client.py 93.12% <0.00%> (-1.06%) ⬇️
wandb/sdk/wandb_run.py 91.16% <0.00%> (-0.19%) ⬇️
wandb/sdk/lib/git.py 79.01% <0.00%> (ø)
wandb/integration/tensorboard/monkeypatch.py 92.59% <0.00%> (+2.46%) ⬆️
wandb/sdk/internal/meta.py 90.79% <0.00%> (+3.06%) ⬆️

@kptkin kptkin changed the title check this add better error handling when tearing down service Aug 24, 2022
@kptkin kptkin marked this pull request as ready for review August 24, 2022 04:38
@kptkin kptkin requested a review from a team August 24, 2022 04:38
@kptkin kptkin merged commit 6f330d4 into master Aug 26, 2022
@kptkin kptkin deleted the debug-colab branch August 26, 2022 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants