Disallow creating a grain activation before a silo is active#9612
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR strengthens the grain activation creation logic by restricting it to only occur when the silo is in the Active state, rather than just checking if it's not terminating. This ensures grain activations cannot be created during intermediate states like Created or Joining.
Key Changes:
- Modified the condition for allowing grain activation creation from
!IsTerminating()to explicitly checking forSiloStatus.Active - Updated error handling to reflect the new constraint, checking for
not SiloStatus.Activeinstead of just terminating states - Renamed and updated the logging method to reflect the broader condition
| var isTerminating = self._siloStatusOracle.CurrentStatus.IsTerminating(); | ||
| if (isTerminating) | ||
| var status = self._siloStatusOracle.CurrentStatus; | ||
| var isTerminating = status.IsTerminating(); |
There was a problem hiding this comment.
[nitpick] The variable isTerminating is assigned on line 165 but is no longer used within the if block on lines 166-169. It's only used later on line 179. Consider moving the assignment closer to where it's used, or if it's intentionally kept here for clarity, this is fine but could be optimized.
| @@ -391,10 +392,9 @@ private readonly struct SiloAddressLogValue(SiloAddress silo) | |||
| private partial void LogDebugDeactivateAllActivations(); | |||
|
|
|||
| [LoggerMessage( | |||
There was a problem hiding this comment.
The new log method LogDebugUnableToCreateActivationWhenNotActive is missing an EventId attribute, while the similar method LogDebugUnableToCreateActivation on line 400 has EventId = (int)ErrorCode.CatalogNonExistingActivation2. Consider adding an appropriate EventId for consistency and to aid in log analysis.
| [LoggerMessage( | |
| [LoggerMessage( | |
| EventId = (int)ErrorCode.Catalog_SiloNotActive, |
1 participant
Opening to run tests in CI. I expect tests to fail
Microsoft Reviewers: Open in CodeFlow