Copy fo-dicom serialization code to fix upload failure #1450
Conversation
pengchen0692
changed the title
src/Microsoft.Health.Dicom.Core/Extensions/JsonSerializerOptionsExtensions.cs
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Core/Extensions/JsonSerializerOptionsExtensions.cs
Show resolved
Hide resolved
src/FellowOakDicom.Serialization.Forked/FellowOakDicom.Serialization.Forked.csproj
Outdated
Show resolved
Hide resolved
src/FellowOakDicom.Serialization.Forked/Properties/AssemblyInfo.cs
Outdated
Show resolved
Hide resolved
src/FellowOakDicom.Serialization.Forked/FellowOakDicom.Serialization.Forked.csproj
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,16 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
There was a problem hiding this comment.
project name starts with Microsoft.Health so that can be signed automatically in workspace-platform
| $password_raw = '$(tenant-admin-user-password)' | ||
| $password_raw = | ||
| @" | ||
| $(tenant-admin-user-password) |
There was a problem hiding this comment.
it fix pipeline failure -- password contains single quote which breaks script. I mentioned that in team channel. :)
| @@ -0,0 +1,15 @@ | |||
| # Analyzers | |||
There was a problem hiding this comment.
Oh, is the .editorconfig still needed if AnalysisMode is None?
There was a problem hiding this comment.
yeah... it still fail even with AnalysisMode as None
| dataSet.Add(tag, "InvalidISValue"); | ||
| DicomWebException exception = await Assert.ThrowsAsync<DicomWebException>(() => _client.StoreAsync(dicomFile)); | ||
| Assert.Equal(HttpStatusCode.Conflict, exception.StatusCode); | ||
| } |
| { | ||
| DicomTag tag = DicomTag.StageNumber; | ||
|
|
||
| await CleanupExtendedQueryTag(tag); |
There was a problem hiding this comment.
Will the usage of StageNumber in both tests cause race conditions?
There was a problem hiding this comment.
As far as I know, such tests should be run sequentially
There was a problem hiding this comment.
I will fix it later.
Description
When VR is DS/IS, fo-dicom serialization treat value as number. Customer dicom files could be old and contains invalid numbers. Our service blocks such file upload due to the serialization exception.
The fix is on fo-dicom side. The PR on has been approved, currently blocked by fo-dicom pipeline, which should be fixed soon.
As fo-dicom doesn't release often(fo-dicom/fo-dicom#1357), copy the code into our repository as short term solution, meanwhile exploring long term solution.
Related issues
Addresses AB#88124.
Testing
All tests pass