APIView pipeline job must update comment when APIView job fails

[mikekistler]

The APIView PR pipeline job posts a comment in the PR to say that it has begun generation of an APIView:

But if APIView generation fails, the comment is not always updated to indicate that processing failed and why.

The APIView pipeline job should ensure that this comment is updated, whatever the outcome of the processing.

Also, the PR Check should not report "Succeeded" when it has actually failed.

[chidozieononiwu]

The APIView PR pipeline job posts a comment in the PR to say that it has begun generation of an APIView:

But if APIView generation fails, the comment is not always updated to indicate that processing failed and why.

The APIView pipeline job should ensure that this comment is updated, whatever the outcome of the processing.

The comment is updated if APIView generation fails. The only case where it's not updated is if all the APIViews are skipped.

[chidozieononiwu]

Another thing to keep in mind is that the pipeline might generate SDK and Swagger APIReview. While it aggregates all the results in the pipeline under the Generated APIReview Comment

The success report in the swagger validation pipeline refers only to the Swagger APIReview generation, so if an SDK review generation should fail this will still apprear as a success.

[chidozieononiwu]

I think I have identified the actual problem cases.
Case 1: SwaggerAPIView failed but Failure case was not reported as in openapi-env-test/azure-rest-api-specs#3638 , In this case I will report the failure in the comment.
Case2: Swager API View was not generated but Swagger Job still appeared as successful as in Azure/azure-rest-api-specs#22862 , in this case I will add information stating that no swagger generation was carried out.
case 3: Swagger APIView was generated but Generated ApiView comment was not updated as in Azure/azure-rest-api-specs#23105

[chidozieononiwu]

I have fixed the first two cases. And I have added logging so as to be able to track down exactly where and how events are getting missing, dropped or not picked up.

One approach suggested by @praveenkuttappan is to add the comment directly from APIView service using the DetectApiChanges endpoint and setting commentOnPr to true. We will have to format the comment into the existing table structure, then we can disable the unifiedPipelines -> eventhubs -> kebabbot approach for adding the Generated APIView comment.

@weshaggard @maririos @praveenkuttappan

[weshaggard]

I would like to avoid having the APIView service knowing about the specs PR and figuring out how to update those type of comments. I know we do it for the SDK repos currently but I'd like to eliminate that at some point as well, if possible.

Is the issue that we are generating the APIView's async and we don't update the comment until it is finished? I thought APIView was always returning a URL even if it is wasn't ready yet and we should be able to add that to the comment.

[chidozieononiwu]

A event is pushed to eventhub for each APIView generation attempted, with the result (success or fail) of the attempt. The kebab bot picks up this event then writes the comment to Github. The issue here is one where the event was pushed to eventhub but the kebab bot did not write any comment.

[weshaggard]

Yeah that seems like a bug we should be trying to handle in the kebab bot as opposed to adding a new ApiView dependency on it.