Skip to content

Conversation

aaronburtle
Copy link
Contributor

@aaronburtle aaronburtle commented Mar 14, 2024

Why make this change?

Related to #2064

Closes #2064

We are getting error messaging associated with making edits to the config file due to a file watcher that is constantly looking for changes to the config file as part of the initial pieces of hot reload. Until hot reload is ready this is not error messaging that we want to be made visible.

What is this change?

Add a function to the runtimeconfig that checks if we are in a hot reloadable scenario. For now, this will always return false, and when we want to make the hot reload feature available, then the logic of this function can change to accommodate those updates.

How was this tested?

Run manually to verify we are not file watching.

NOTE: testing for hot reload will need to be turned back on before we enable it as a feature, we have an issue to track the unit tests here: #2109

Sample Request(s)

No request is needed, simply start dab, make a change to the config file and save it, you will no longer see the error messages.

To verify the differences in the error messages and validate that they are consistent and what we expect, you can do the following:

  1. Start dab (using the current version without this PR's changes) as normal, noting which config file you are using and its file location.
  2. Make a rest request using the path from the config during startup (ie: "path": "/api") https://localhost:5001/api/...
  3. While dab is running, open that same config file in an editor that will only hold a write lock while saving, such as Notepad++ and change the path (ie: "path": "/rest")
  4. Save your changes
  5. Note the error messages that indicate that hot reloading is not possible due to the file being in use by another process
  6. Make a rest request using the new path from the saved config file https://localhost:5001/rest/...
  7. Note that hot reloading did in fact work and your rest request works with the new path
  8. Open the config file in another editor which will hold the write lock while the file is open, such as Visual Studio Code and change the path (ie: change it back to "path": "/api")
  9. Note the many error messages that indicate hot reloading is not possible due to the file being in use by another process
  10. Make a rest request using the newest path from the changes you just made. https://localhost:5001/api/...
  11. Note that hot reloading can not work since the write lock was never relinquished by the editor.

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this to ensure we don't experience the file contention error when not using hot reload yet. Small favor to ask before you merge in:

  • Were you able to get consistent repro of the error message you get before this change?
  • Can you provide those repro steps in the PR description so that we can check locally to confirm that the error no longer appears?

@aaronburtle
Copy link
Contributor Author

  • Were you able to get consistent repro of the error message you get before this change?
  • Can you provide those repro steps in the PR description so that we can check locally to confirm that the error no longer appears?

Yes, I was, and I will add those steps.

@aaronburtle
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@abhishekkumams abhishekkumams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix.

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.

[Bug]: Unable to hot reload runtime section since file is being used by another process
3 participants