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

Do not allow Mode to Hot-Reload #2478

Merged
merged 9 commits into from
Nov 27, 2024
Merged

Conversation

aaronburtle
Copy link
Contributor

Why make this change?

Closes #2457

What is this change?

We add in an optional parameter to the TryLoadConfig function so that when we Hot-Reload, we can pass along the Mode. When that parameter is included (and therefore non null), we overwrite the Mode of the RuntimeConfig to match the value of the previous RuntimeConfig. This means that this Mode needs to have a public setter however, which we now utilize to overwrite.

How was this tested?

Modified our Unit test to provide coverage.

Sample Request(s)

Any hot-reload that tries to change the mode will demonstrate the functionality. So start the engine in dev mode, so that the filewatcher will pick up any changes as we dont currently support hot-reload in production, then change the Host Mode from development to production, and note that the RuntimeConfig in-memory object is still in dev mode.

@aaronburtle aaronburtle self-assigned this Nov 26, 2024
@aaronburtle aaronburtle added the bug Something isn't working label Nov 26, 2024
@aaronburtle aaronburtle added this to the 1.3 milestone Nov 26, 2024
@aaronburtle
Copy link
Contributor Author

/azp run

@abhishekkumams
Copy link
Contributor

abhishekkumams commented Nov 26, 2024

Thanks @aaronburtle for quick fix.
We should also need to log error in console if user tries to hot-reload hostmode.

@abhishekkumams abhishekkumams force-pushed the dev/aaronburtle/ModeHotReload branch from 0d1a9e0 to 450eb32 Compare November 26, 2024 11:09
@abhishekkumams
Copy link
Contributor

/azp run

@abhishekkumams
Copy link
Contributor

/azp run

Copy link
Contributor

@sezal98 sezal98 left a comment

Choose a reason for hiding this comment

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

lgtm

src/Config/FileSystemRuntimeConfigLoader.cs Show resolved Hide resolved
src/Config/FileSystemRuntimeConfigLoader.cs Outdated Show resolved Hide resolved
src/Config/FileSystemRuntimeConfigLoader.cs Outdated Show resolved Hide resolved
@aaronburtle
Copy link
Contributor Author

/azp run

@aaronburtle
Copy link
Contributor Author

/azp run

@aaronburtle aaronburtle merged commit 053fe83 into main Nov 27, 2024
7 checks passed
@aaronburtle aaronburtle deleted the dev/aaronburtle/ModeHotReload branch November 27, 2024 21:55
abhishekkumams added a commit that referenced this pull request Nov 28, 2024
## Why make this change?

Closes #2457

## What is this change?

We add in an optional parameter to the `TryLoadConfig` function so that
when we Hot-Reload, we can pass along the `Mode`. When that parameter is
included (and therefore non null), we overwrite the Mode of the
`RuntimeConfig` to match the value of the previous `RuntimeConfig`. This
means that this Mode needs to have a public setter however, which we now
utilize to overwrite.

## How was this tested?

Modified our Unit test to provide coverage.

## Sample Request(s)

Any hot-reload that tries to change the mode will demonstrate the
functionality. So start the engine in dev mode, so that the filewatcher
will pick up any changes as we dont currently support hot-reload in
production, then change the Host Mode from development to production,
and note that the RuntimeConfig in-memory object is still in dev mode.

---------

Co-authored-by: Abhishek Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Hot Reload Not Re-enabled When Switching from Production to Development Mode
5 participants