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: When a python measurement starts discovery service, hide discovery service logs #343

Merged
merged 10 commits into from
Jul 31, 2023

Conversation

vigkre
Copy link

@vigkre vigkre commented Jul 27, 2023

What does this Pull Request accomplish?

  • This PR redirects the logs to /dev/null (hide logs) when a python measurement starts discovery service.
  • Before:

image

  • After:

image

  • Handle FileNotFoundError when wrong discovery service .exe path is specified for subprocess.popen function.
  • Add tests to register service function to handle FileNotFoundError exception.

Why should this Pull Request be merged?

  • Implements Task 2462367: When a python measurement starts discovery service, it should not show logs from discovery service to the user.

What testing has been done?

  • Verified discovery service logs are not displayed.
  • While the discovery service logs are hidden, it might hide other errors that might arise while starting discovery service through python measurement. To test this case:
    - Specified wrong discovery service .exe path to subprocess.popen function and noticed the log error is captured in the console:

image

@github-actions
Copy link

github-actions bot commented Jul 27, 2023

Test Results

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

Results for commit 5f97556. ± Comparison against base commit dbbee49.

This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.
tests.unit.test_discovery_client ‑ test___discovery_service_unavailable___register_service_registration_failure
tests.unit.test_discovery_client ‑ test___discovery_service_exe_unavailable___register_service___registration_failure

♻️ This comment has been updated with latest results.

@vigkre vigkre force-pushed the users/vikram/hide-discovery-service-console-logs branch from 9a090b3 to f280a25 Compare July 27, 2023 14:44
@vigkre vigkre requested a review from bkeryan July 27, 2023 15:01
@vigkre vigkre requested review from bkeryan and mshafer-NI July 27, 2023 15:59
…if service is not registered with disc service
@vigkre vigkre merged commit c89b733 into main Jul 31, 2023
@dixonjoel dixonjoel deleted the users/vikram/hide-discovery-service-console-logs 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.

6 participants