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

Retrieve original file properties when requested #3360

Merged
merged 14 commits into from
Feb 16, 2024
Merged

Conversation

bcarthic
Copy link
Contributor

@bcarthic bcarthic commented Feb 13, 2024

Description

Retrieve original file properties when requested

Related issues

Addresses AB#115194

Testing

Added new integration tests

@bcarthic bcarthic requested a review from a team as a code owner February 13, 2024 00:13
@@ -383,6 +386,27 @@ private async Task<InstanceMetadata> GetInstanceMetadata(InstanceIdentifier inst
instanceIdentifier.StudyInstanceUid,
instanceIdentifier.SeriesInstanceUid,
instanceIdentifier.SopInstanceUid,
isOriginalVersion: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

is this correct? that is it always true?

Copy link
Contributor

Choose a reason for hiding this comment

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

is there any way we can make this safer based on what feature supports requesting original vs not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For frames, we always get from original even after update. only time we do get the latest is when they need transcoding.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think we should do something here to make the contract clearer vs passing these booleans around. We've run into easy bugs in the past with this - today it's very clear that only in some very specific instances do we even allow for either or, and in some we only allow original, and in some we only allow latest. We should find away to centrally describe this contract

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the variable to initialversion, which can be either original or latest depends on update happened or not

@bcarthic bcarthic merged commit fcc2db2 into main Feb 16, 2024
17 checks passed
@bcarthic bcarthic deleted the bcarthic/fixfileprop branch February 16, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants