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.

Sign up for GitHub

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

Jump to bottom

Return bad request on too many surrogate ids #3838

Merged
merged 4 commits into from
May 6, 2024
Merged

Return bad request on too many surrogate ids #3838

merged 4 commits into from
May 6, 2024

Conversation

SergeyGaluzo
Copy link
Contributor

@SergeyGaluzo SergeyGaluzo commented May 3, 2024 •
edited
Loading

When >80K resources with the same last updated are ingested, code fails with
Current - 500
After fix - 400 with good message

fixes https://microsofthealth.visualstudio.com/Health/_workitems/edit/120027/

@SergeyGaluzo SergeyGaluzo added Enhancement Enhancement on existing functionality. Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Schema Version unchanged labels May 3, 2024
@SergeyGaluzo SergeyGaluzo added this to the S138 milestone May 3, 2024
@SergeyGaluzo SergeyGaluzo requested a review from a team as a code owner May 3, 2024 21:58
@SergeyGaluzo SergeyGaluzo changed the title Raise bad request on too many surrogate ids Return bad request on too many surrogate ids May 4, 2024
@microsoft microsoft deleted a comment from azure-pipelines bot May 5, 2024
@microsoft microsoft deleted a comment from azure-pipelines bot May 5, 2024
@SergeyGaluzo SergeyGaluzo enabled auto-merge (squash) May 5, 2024 01:17
mikaelweave
mikaelweave reviewed May 6, 2024
View reviewed changes
Copy link
Contributor

@mikaelweave mikaelweave left a comment

Choose a reason for hiding this comment

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

Some questions but in general PR looks good.

Will approve after hearing back about change near:
// dedup by last updated - take first version for single last updated, prefer large version.

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs Show resolved Hide resolved
src/Microsoft.Health.Fhir.SqlServer/Features/Operations/Import/ImportProcessingJob.cs Show resolved Hide resolved
src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs Show resolved Hide resolved
mikaelweave
mikaelweave approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@mikaelweave mikaelweave left a comment

Choose a reason for hiding this comment

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

🦾

@SergeyGaluzo SergeyGaluzo merged commit 7aebd75 into main May 6, 2024
44 of 45 checks passed
@SergeyGaluzo SergeyGaluzo deleted the users/sergal/tests branch May 6, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikaelweave mikaelweave mikaelweave approved these changes

Assignees
No one assigned
Labels
Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Enhancement Enhancement on existing functionality. Schema Version unchanged
Projects
None yet
Milestone
S138
Development

Successfully merging this pull request may close these issues.
2 participants
@SergeyGaluzo @mikaelweave