Skip to content

Conversation

@samchris007
Copy link
Contributor

@samchris007 samchris007 commented Sep 22, 2023

What does this Pull Request accomplish?

When attempting to open the discovery service key file after a fresh install of MeasurementLink (i.e., folders such as Discovery and Logs are not present in C:\ProgramData\National Instruments\MeasurementLink\), the winerror.ERROR_PATH_NOT_FOUND exception was thrown because these directories were not present at the start. This exception hadn't been handled in _open_key_file(). This unhandled error resulted in the exception being thrown in the command prompt during the first run of a measurement service alone.

To fix this issue, I have made the following changes based on the suggestions provided by @bkeryan.

  • Updated _open_key_file to translate ERROR_PATH_NOT_FOUND to FileNotFoundError, which is handled by its caller (_start_service)
  • Updated _start_service to catch OSError.
  • Additionally, in _open_key_file(), I have translated all unexpected win32file.error codes to WindowsError

Why should this Pull Request be merged?

Fixes Bug 2514643: Auto-launching discovery service fails after clean install (GitHub Issue #369)

What testing has been done?

Added/Updated unit tests to test the fix.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2023

Test Results

     12 files  ±  0       12 suites  ±0   2m 18s ⏱️ -1s
   217 tests +  2     188 ✔️ +2    29 💤 ±  0  0 ±0 
2 592 runs  +24  2 228 ✔️ +8  364 💤 +16  0 ±0 

Results for commit 9cd18da. ± Comparison against base commit d53f08d.

♻️ This comment has been updated with latest results.

@samchris007 samchris007 force-pushed the users/sam/fix-discoveryservicelaunch-onfreshInstall branch from 5e8b73b to 75262fa Compare September 22, 2023 12:54
@dixonjoel
Copy link
Collaborator

If we wanted to reproduce this locally and test the fix without having to do a fresh install, would you recommend just deleting the 'Logs' or 'Discovery' folder in programdata?

@samchris007
Copy link
Contributor Author

samchris007 commented Sep 22, 2023

@dixonjoel

If we wanted to reproduce this locally and test the fix without having to do a fresh install, would you recommend just deleting the 'Logs' or 'Discovery' folder in programdata?

Yes, we can terminate any existing running discovery service, delete the Discovery folder from C:\ProgramData\National Instruments\MeasurementLink\ and then test this use case

Note: Running without any debuggers makes it more easily reproducible

@samchris007 samchris007 force-pushed the users/sam/fix-discoveryservicelaunch-onfreshInstall branch from 9cb7cf5 to 81444bc Compare September 25, 2023 14:04
@samchris007 samchris007 merged commit d8b9336 into main Sep 25, 2023
samchris007 added a commit that referenced this pull request Sep 25, 2023
… a fresh install of MeasurementLink (#397)

* Fix launching of discovery service during fresh install
samchris007 added a commit that referenced this pull request Sep 25, 2023
… a fresh install of MeasurementLink (#397)

* Fix launching of discovery service during fresh install
dixonjoel pushed a commit that referenced this pull request Sep 25, 2023
…y service after a fresh install of MeasurementLink (#407)

Fix pywintypes error during the launch of the discovery service after a fresh install of MeasurementLink (#397)

* Fix launching of discovery service during fresh install
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.

5 participants