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

discovery_client: Fix auto-launching discovery service producing a ResourceWarning #344

Merged
merged 4 commits into from
Aug 1, 2023

Conversation

vigkre
Copy link

@vigkre vigkre commented Jul 28, 2023

What does this Pull Request accomplish?

  • Save the reference of the subprocess.popen object to a global variable.
  • This prevents leakage of subprocess which in turn denotes that the subprocess is still running and produces a ResourceWarning.

Why should this Pull Request be merged?

What testing has been done?

  1. Ran example (nidaqmx_analog_input) service and make sure it is registered with the discovery service.
  2. Used Task Manager to kill the discovery service.
  3. Manually launched the example service with poetry run python measurement.py -v.
  4. Verified ResourceWarning doesn't happen anymore.
  5. Repeated step 1 to 4 for other examples as well.
  6. For other examples, in step 3 used poetry run python -X dev measurement.py to reproduce the ResourceWarning.

@github-actions
Copy link

github-actions bot commented Jul 28, 2023

Test Results

     12 files  ±0       12 suites  ±0   18m 3s ⏱️ + 1m 7s
   180 tests ±0     180 ✔️ ±0  0 💤 ±0  0 ±0 
2 148 runs  ±0  2 148 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 49a2a81. ± Comparison against base commit c89b733.

♻️ This comment has been updated with latest results.

@vigkre vigkre requested a review from bkeryan July 29, 2023 14:31
…nto users/vikram/fix-disc-service-resource-warning
@vigkre vigkre merged commit 3e0ded3 into main Aug 1, 2023
@dixonjoel dixonjoel deleted the users/vikram/fix-disc-service-resource-warning branch August 16, 2023 13:15
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.

Auto-launching discovery service produces a ResourceWarning
3 participants