EventGrid - AzureHealthcareApis swagger update for GA #20918

jnlycklama · 2022-09-28T18:40:09Z

Data Plane API - Pull Request

Add new event types for the DICOM service to healthcareapis eventgrid schema.
The DICOM service is a child service of a Healthcare Data Services workspace, similar to a FHIR service which is sending the existing event types.

API Info: The Basics

The DICOM service is a child service of a Healthcare Data Services workspace, similar to a FHIR service which is sending the existing event types.

Is this review for (select one):

a private preview
a public preview
[ x] GA release

Change Scope

^{This section will help us focus on the specific parts of your API that are new or have been modified.
Please share a link to the design document for the new APIs, a link to the previous Open API document (swagger) if applicable, and the root paths that have been updated.}

Design Document: https://microsofthealth.visualstudio.com/Health/_git/health-paas-docs?path=/specs/DICOM/DICOMevents.md&_a=preview
Previous Open API Doc:
Updated paths: Added 2 new event types

❔Got questions? Need additional info?? We are here to help!

Contact us!

The Azure API Review Board is dedicated to helping you create amazing APIs. You can read about our mission and learn more about our process on our wiki.

💬 Teams Channel
💌 email

Click here for links to tools, specs, guidelines & other good stuff

Tooling

Open API validation tools were run on this PR. Go here to see how to fix errors
Spectral Linting
Open API Hub

Guidelines & Specifications

Helpful Links

Azure DevTools Wiki

openapi-workflow-bot · 2022-09-28T18:40:11Z

Hi, @jnlycklama Thanks for your PR. I am workflow bot for review process. Here are some small tips.

Please ensure to do self-check against checklists in first PR comment.

PR assignee is the person auto-assigned and responsible for your current PR reviewing and merging.

For specs comparison cross API versions, Use API Specs Comparison Report Generator

If there is CI failure(s), to fix CI error(s) is mandatory for PR merging; or you need to provide justification in PR comment for explanation. How to fix?

Any feedback about review process or workflow bot, pls contact swagger and tools team. [email protected]

openapi-pipeline-app · 2022-09-28T18:40:13Z

Swagger Validation Report

️️✔️BreakingChange succeeded [Detail] [Expand]

There are no breaking changes.

compared swaggers (via Oad v0.10.1)]	new version	base version
HealthcareApis.json	2018-01-01(91e1da8)	2018-01-01(main)

️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]

There are no breaking changes.

️️✔️CredScan succeeded [Detail] [Expand]

There is no credential detected.

️❌LintDiff: 2 Errors, 0 Warnings failed [Detail]

[must fix]The following errors/warnings are introduced by current PR:

Rule	Message	Related RPC [For API reviewers]
`❌` AutoRest exception	"location":"specification/eventgrid/data-plane/readme.md", "tag":"package-2018-01", "details":"swagger-document/individual/schema-validator - FAILED"
`❌` AutoRest exception	"location":"specification/eventgrid/data-plane/readme.md", "tag":"package-2018-01", "details":"Error: [OperationAbortedException] Error occurred. Exiting."

️️✔️Avocado succeeded [Detail] [Expand]

Validation passes for Avocado.

️️✔️ApiReadinessCheck succeeded [Detail] [Expand]

️⚠️~[Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]

API Test is not triggered due to precheck failure. Check pipeline log for details.

️️✔️ModelValidation succeeded [Detail] [Expand]

Validation passes for ModelValidation.

️️✔️SemanticValidation succeeded [Detail] [Expand]

Validation passes for SemanticValidation.

️️✔️PoliCheck succeeded [Detail] [Expand]

Validation passed for PoliCheck.

️️✔️PrettierCheck succeeded [Detail] [Expand]

Validation passes for PrettierCheck.

️️✔️SpellCheck succeeded [Detail] [Expand]

Validation passes for SpellCheck.

️️✔️Lint(RPaaS) succeeded [Detail] [Expand]

Validation passes for Lint(RPaaS).

️️✔️CadlValidation succeeded [Detail] [Expand]

Validation passes for CadlValidation.

️️✔️PR Summary succeeded [Detail] [Expand]

Validation passes for Summary.

_{Posted by Swagger Pipeline | How to fix these errors?}

openapi-pipeline-app · 2022-09-28T18:40:13Z

Swagger Generation Artifacts

️️✔️ApiDocPreview succeeded [Detail] [Expand]

 Please click here to preview with your @microsoft account.

️️✔️SDK Breaking Change Tracking succeeded [Detail] [Expand]

Breaking Changes Tracking

️️✔️ azure-sdk-for-net-track2 succeeded [Detail] [Expand]

️✔️Succeeded [Logs]Release - Generate from 8c9845c. SDK Automation 14.0.0

command	pwsh ./eng/scripts/Automation-Sdk-Init.ps1 ../azure-sdk-for-net_tmp/initInput.json ../azure-sdk-for-net_tmp/initOutput.json
warn	File azure-sdk-for-net_tmp/initOutput.json not found to read
command	pwsh ./eng/scripts/Invoke-GenerateAndBuildV2.ps1 ../azure-sdk-for-net_tmp/generateInput.json ../azure-sdk-for-net_tmp/generateOutput.json

️✔️Azure.Messaging.EventGrid [View full logs] [Release SDK Changes]
- Azure.Messaging.EventGrid.4.12.0-alpha.20221104.1.nupkg
```
info	[Changelog]
```

_{Posted by Swagger Pipeline | How to fix these errors?}

openapi-pipeline-app · 2022-09-28T18:40:14Z

Swagger pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

openapi-workflow-bot · 2022-09-28T18:43:51Z

Hi @jnlycklama, Your PR has some issues. Please fix the CI sequentially by following the order of Avocado, semantic validation, model validation, breaking change, lintDiff. If you have any questions, please post your questions in this channel https://aka.ms/swaggersupport.

Task	How to fix	Priority
Avocado	Fix-Avocado	High
Semantic validation	Fix-SemanticValidation-Error	High
Model validation	Fix-ModelValidation-Error	High
LintDiff	Fix-LintDiff	high

If you need further help, please feedback via swagger feedback.

openapi-workflow-bot · 2022-09-28T18:46:54Z

Hi @jnlycklama, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review.
Action: To initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
If you want to know the production traffic statistic, please see ARM Traffic statistic.
If you think it is false positive breaking change, please provide the reasons in the PR comment, report to Swagger Tooling Team via https://aka.ms/swaggerfeedback.
Note: To avoid breaking change, you can refer to Shift Left Solution for detecting breaking change in early phase at your service code repository.

JoshLove-msft · 2022-09-28T20:57:11Z

...fication/eventgrid/data-plane/Microsoft.HealthcareApis/stable/2018-01-01/HealthcareApis.json

+          "description": "Host name of the DICOM account for this image.",
+          "type": "string"
+        },
+        "dicomServiceSequence": {


Is the sequence purely a service concept? i.e. is not called out in the DICOM spec? Is this different than the Sequence defined here - https://dicom.nema.org/medical/Dicom/2016c/output/chtml/part03/sect_5.2.html

I called this sequence based on your comment in the private PR. It was previously called changefeedId.

Its not the same sequence from that link. Its essentially the sequence number for changes occurred in the dicom service. Each dicom service keeps a change log. When any image is added or deleted from the service, that change gets added to the change log with a new sequence number. This change log is exposed through our Change Feed API https://github.com/microsoft/dicom-server/blob/main/docs/concepts/change-feed.md , which is specific to our service and not part of the DICOM standard.

I like the change to sequence because it is more generic, however I don't want to get it confused with this kind of Sequence https://dicom.nema.org/medical/Dicom/2016c/output/chtml/part03/sect_5.2.html . What do you think?

Yeah, I think SequenceNumber is a bit less ambiguous even aside from the Dicom naming conflict. Is the Change Feed API already GA?

Yeah the Changefeed API is GA in our service, it was included in the initial GA release of the DICOM service in March.

I will go ahead and change to property to sequenceNumber

...fication/eventgrid/data-plane/Microsoft.HealthcareApis/stable/2018-01-01/HealthcareApis.json

jianyexi · 2022-09-29T06:17:52Z

/azp run

azure-pipelines · 2022-09-29T06:18:01Z

Azure Pipelines successfully started running 1 pipeline(s).

JoshLove-msft

LGTM!

JoshLove-msft · 2022-10-06T21:53:58Z

@jnlycklama has informed me that the service is not fully rolled out and would prefer that we not merge this until they let us know they are ready.

ghost · 2022-10-23T16:02:45Z

Hi, @jnlycklama. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

lmazuel · 2022-11-04T20:22:41Z

Confirmed by @JoshLove-msft as ready to merge, CI failure is irrelevant

* add new dicom event types * Remove dicom prefix * serviceSequence -> sequenceNumber * fix prettier errors * Add example files to main swagger

add new dicom event types

87b7f58

jnlycklama requested review from Kishp01 and ahamad-MS as code owners September 28, 2022 18:40

openapi-assignment-bot bot assigned lmazuel Sep 28, 2022

ghost added the Event Grid label Sep 28, 2022

openapi-pipeline-app bot added the CI-FixRequiredOnFailure label Sep 28, 2022

AzureRestAPISpecReview added BreakingChangeReviewRequired data-plane labels Sep 28, 2022

azure-sdk mentioned this pull request Sep 28, 2022

[AutoPR Azure.Messaging.EventGrid] EventGrid - AzureHealthcareApis swagger update for GA azure-sdk/azure-sdk-for-net#988

Closed

JoshLove-msft reviewed Sep 28, 2022

View reviewed changes

...fication/eventgrid/data-plane/Microsoft.HealthcareApis/stable/2018-01-01/HealthcareApis.json Outdated Show resolved Hide resolved

AzureRestAPISpecReview removed the BreakingChangeReviewRequired label Sep 29, 2022

jnlycklama added 2 commits September 29, 2022 10:18

Remove dicom prefix

d561b16

serviceSequence -> sequenceNumber

94ac34f

JoshLove-msft approved these changes Sep 29, 2022

View reviewed changes

JoshLove-msft mentioned this pull request Sep 30, 2022

Board Review: Event Grid new system events Azure/azure-sdk#4893

Closed

fix prettier errors

d7c665a

JoshLove-msft added the DoNotMerge <valid label in PR review process> use to hold merge after approval label Oct 6, 2022

ghost added the no-recent-activity label Oct 23, 2022

Add example files to main swagger

91e1da8

ghost removed the no-recent-activity label Nov 4, 2022

lmazuel merged commit 8c9845c into Azure:main Nov 4, 2022

azure-sdk mentioned this pull request Nov 4, 2022

[ReleasePR Azure.Messaging.EventGrid] change the event type of tags from string to object Azure/azure-sdk-for-net#28153

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EventGrid - AzureHealthcareApis swagger update for GA #20918

EventGrid - AzureHealthcareApis swagger update for GA #20918

jnlycklama commented Sep 28, 2022

openapi-workflow-bot bot commented Sep 28, 2022

openapi-pipeline-app bot commented Sep 28, 2022 •

edited

Loading

openapi-pipeline-app bot commented Sep 28, 2022 •

edited

Loading

Breaking Changes Tracking

openapi-pipeline-app bot commented Sep 28, 2022 •

edited

Loading

openapi-workflow-bot bot commented Sep 28, 2022

openapi-workflow-bot bot commented Sep 28, 2022

JoshLove-msft Sep 28, 2022

jnlycklama Sep 29, 2022

JoshLove-msft Sep 29, 2022 •

edited

Loading

jnlycklama Sep 29, 2022

jianyexi commented Sep 29, 2022

azure-pipelines bot commented Sep 29, 2022

JoshLove-msft left a comment

JoshLove-msft commented Oct 6, 2022

ghost commented Oct 23, 2022

lmazuel commented Nov 4, 2022

EventGrid - AzureHealthcareApis swagger update for GA #20918

EventGrid - AzureHealthcareApis swagger update for GA #20918

Conversation

jnlycklama commented Sep 28, 2022

Data Plane API - Pull Request

API Info: The Basics

Change Scope

❔Got questions? Need additional info?? We are here to help!

Tooling

Guidelines & Specifications

Helpful Links

openapi-workflow-bot bot commented Sep 28, 2022

openapi-pipeline-app bot commented Sep 28, 2022 • edited Loading

Swagger Validation Report

openapi-pipeline-app bot commented Sep 28, 2022 • edited Loading

Swagger Generation Artifacts

Breaking Changes Tracking

openapi-pipeline-app bot commented Sep 28, 2022 • edited Loading

openapi-workflow-bot bot commented Sep 28, 2022

openapi-workflow-bot bot commented Sep 28, 2022

JoshLove-msft Sep 28, 2022

Choose a reason for hiding this comment

jnlycklama Sep 29, 2022

Choose a reason for hiding this comment

JoshLove-msft Sep 29, 2022 • edited Loading

Choose a reason for hiding this comment

jnlycklama Sep 29, 2022

Choose a reason for hiding this comment

jianyexi commented Sep 29, 2022

azure-pipelines bot commented Sep 29, 2022

JoshLove-msft left a comment

Choose a reason for hiding this comment

JoshLove-msft commented Oct 6, 2022

ghost commented Oct 23, 2022

lmazuel commented Nov 4, 2022

openapi-pipeline-app bot commented Sep 28, 2022 •

edited

Loading

openapi-pipeline-app bot commented Sep 28, 2022 •

edited

Loading

openapi-pipeline-app bot commented Sep 28, 2022 •

edited

Loading

JoshLove-msft Sep 29, 2022 •

edited

Loading