Fix: Retrieve Sandbox and Deployment IDs from CLI (EOSU-668) #1186
Conversation
JessTello
commented
Dec 4, 2025
JessTello
commented
- Added new class to fetch Sandbox ID and Deployment ID from CLI arguments
- Replaced previous logic that only read values from JSON after initialization
- Ensures correct IDs are used when running builds in different environments
- Added new class to fetch Sandbox ID and Deployment ID from CLI arguments - Replaced previous logic that only read values from JSON after initialization - Ensures correct IDs are used when running builds in different environments
| // Save values globally for later access | ||
| // -------------------------------------------------------------------- | ||
| EOSOverrideState.DeploymentIdOverride = deploymentArg; | ||
| EOSOverrideState.SandboxIdOverride = sandboxArg; |
There was a problem hiding this comment.
I'm a bit confused about the need for both these override vars and updating the platform config below. If the override vars are used then why is there a need to update the platform config?
With that in mind I would get rid of the override vars and just update the config. This eliminates the chance that someone might read the values from the config without checking the overrides.
There was a problem hiding this comment.
The main reason is that PlatformManager.GetPlatformConfig() is called in several places, and every time this function runs, it restores and reads the values from the JSON file. I would need to do a deeper analysis to understand why it was implemented this way and whether it’s truly necessary. However, to avoid any risk, I decided to take the safer approach and store the values so that EOSManager reads them directly, instead of using the ones that are constantly overwritten when PlatformManager.GetPlatformConfig() is called.
Additionally, with this approach, if the CLI is not used, the platform still falls back to the values from the platform JSON.
There was a problem hiding this comment.
By the way, the reason for applying them now is to prepare for a future refactor of PlatformManager.GetPlatformConfig(). Once that method stops overwriting values, this script will already handle applying them, so the overrides could be removed without extra work. That’s why I didn’t remove the function but added an explicit comment for clarity.
There was a problem hiding this comment.
That's very strange, PlatformManager.GetPlatformConfig() uses the static var s_platformConfig. Once it's been called once it should always use that cached version.
Regardless changing the GetPlatformConfig() function to apply the cmd line args would be safest, which can be done straight after s_platformConfig is assigned. This way whenever GetPlatformConfig() is called then these overrides are applied, even if something were to clear s_platformConfig since it was last called.
There was a problem hiding this comment.
You were right, Matt. Thank you so much for pointing that out. It’s now implemented in the new commit.
- Fixed invalid char in debug log - Applied namespace globally - Replaced nested if-if with if-else - Updated null checks to use string.IsNullOrEmpty - Removed inaccurate and redundant comments
- Ensure overrides are applied before s_platformConfig assignment
