Reservation error handling - #265
Merged
Merged
Conversation
Replaced obsolete ResourceManagerTraceDataHandler in CoreJobHandler.cs with a new, robust version in its own file. The new handler translates core ResourceManager errors to DevPack job resource errors, mapping resource, capacity, and capability IDs to Resource Studio (DOM) equivalents. It handles specific error types and falls back to raw messages for unknown errors. Added deterministic, simulation-backed unit tests to verify translation logic and fallback behavior.
Skyline Quality Gate: ✅ Passed
SonarCloud: new-code dashboard for See the Actions run for full logs. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors and expands the translation of core ResourceManager errors into DevPack job resource errors by extracting ResourceManagerTraceDataHandler into its own file and adding deterministic simulation-backed unit tests to validate the new mappings.
Changes:
- Added an internal
ResourceManagerTraceDataHandlerimplementation that maps specificResourceManagerErrorData.Reasonvalues to typed DevPack job resource errors (including core→DOM resource ID resolution). - Removed the previous placeholder inner handler from
CoreJobHandler. - Added deterministic unit tests covering quarantine/capacity/capability translations and unknown-reason fallback behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| DevPack/API/Handlers/Workflow/Jobs/ResourceManagerTraceDataHandler.cs | New extracted handler that translates core ResourceManager trace errors into DevPack trace errors keyed by reservation ID, including core→DOM resource ID mapping. |
| DevPack/API/Handlers/Workflow/Jobs/CoreJobHandler.cs | Removes the old inner ResourceManagerTraceDataHandler implementation and its incomplete translation logic. |
| DevPack.Tests/Workflow/Jobs/ResourceManagerTraceDataHandlerTests.cs | Adds deterministic simulation-backed tests validating the new translation behavior and fallback behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Added FilePropertySetting for file value management, including attachment handling via IPropertyAttachmentStore. Updated property collections for file support, serialization, and validation (size, count, type). Exposed configurable max document size. Added tests for file properties and error cases. Refactored persistence and cleanup logic for attachments.
- Disallow separator and invalid characters in file names; add unit tests - Track stored files in FilePropertySetting to delete only uploaded files - Track removed file settings in PropertySettingCollection for cleanup - Update cleanup logic to handle current and removed file settings, clear removed list after cleanup - Return persisted instances after create/update for accurate reporting - Add tests for attachment deletion and structured error reporting on upload failure
Builds domResourceIdByCoreId using three error lists via BuildDomResourceIdByCoreId. Passes the mapping to HandleReservationUpdateCausedReservationsToGoToQuarantine for further processing.
- Retain attachments when updating collections with same file property - Copy file content when copying settings between collections - Refine cleanup to delete only orphaned attachments - Update constructors to accept destination collection ID - Add and update tests for attachment retention and copying - Remove explicit attachment deletion on collection removal
Replaced Path methods with explicit handling of '/' and '\' as directory separators and a custom invalid character set for cross-platform consistency. Enhanced validation to reject empty, ".", "..", and control character file names. Added unit tests to verify stripping of directory info and exception handling for invalid names.
…racking removed file settings Co-authored-by: JensVandewalle <102030104+JensVandewalle@users.noreply.github.com>
…eview-suggestion Reconcile property attachments against stored names to prevent orphans on file setting removal
Member
Author
|
@copilot can you revert the last merge that has been done in this branch? |
Co-authored-by: JensVandewalle <102030104+JensVandewalle@users.noreply.github.com>
Contributor
ThomasRemmery
requested changes
Aug 13, 2026
| } | ||
|
|
||
| FilterElement<DomInstance> Filter(Guid coreResourceId) => | ||
| DomInstanceExposers.DomDefinitionId.Equal(SlcResource_StudioIds.Definitions.Resource.Id) |
Contributor
There was a problem hiding this comment.
You could make use of a select query here as you only require the DOM instance ID.
Update SlcResourceStudioHelper with GetResourceStudioFields to fetch partial DOM instances based on selected fields and filters. Refactor ResourceManagerTraceDataHandler to use this method, retrieving only required fields (e.g., Resource_Id) for improved performance. Add null checks and support for SelectedFields and PartialObject types.
Introduce DomInstanceSelectStore to handle partial object select read requests in unit tests, simulating DataMiner Agent behavior. Update SimulatedDms.cs to use this handler before the default DOM handler. Update package references and versions; remove dependency on Skyline.DataMiner.Files.SLNetTypes.
ThomasRemmery
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Classification
Refactor and expand error handling logic by extracting and enhancing the ResourceManagerTraceDataHandler class.
PR Summary
The ResourceManagerTraceDataHandler class has been moved from CoreJobHandler.cs to its own file, made internal, and its functionality has been significantly expanded to provide robust translation of core ResourceManager errors into DevPack job resource errors. Deterministic unit tests have been added to verify the new behavior.
AB#45799