Azure / azure-rest-api-specs

The source for REST API specifications for Microsoft Azure.
MIT License
2.63k stars 5.08k forks source link

[TypeSpec Validation] Show per-rule status in GitHub Checks UI #24999

Open mikeharder opened 1 year ago

mikeharder commented 1 year ago

To make it easier for spec authors to interpret the results, TypeSpecValidation should output the status of each rule directly to the GitHub Checks UI, similar to what we do in TypeSpecRequirement:

https://github.com/Azure/azure-rest-api-specs/blob/8fc3dd4d4582aa2c8201af98e6f1143521cb76fc/eng/scripts/TypeSpec-Requirement.ps1#L107

konrad-jamrozik commented 1 year ago

As discussed with @mikeharder, we also want to have "getting help" link, as captured by this work item:

Which fits in the bigger picture captured here:

@ckairen for the current implementation on how the table is composed, the call site for that is renderUnifiedPipelineCheck.ts / buildCheckResult calling into buildCheckText,

which then calls:

const generateView = compileHandlebarsTemplate<HandlebarCheckGenData>("checker.handlebars", { isDetail: true });

from checkerBuilder.ts which uses the checker.handlebars template.

However, reusing this logic will be challenging - unified pipeline keeps track of the check information based on its own internal Cosmos DB instance with MongoDB interface, and the new TypeSpec validation check is completely detached from it. Hence to reuse this code, you would need to modify it in such a way it can work with the new TypeSpec validation check ignoring most of the data structures used in unified pipeline. The code is tightly coupled to it. For example, the data which is used to populate the table is pulled from the Cosmos DB database, which you don't want to use. We should chat about it.

Overall, I am owner of the pipeline-bot code and somewhat familiar with it, so feel free to reach out and please add me as a reviewer for any changes.

mikeharder commented 1 year ago

@konrad-jamrozik: Are you saying that appendMsg in the logs is something specific to pipeline-bot and not a general-purpose feature to set UI in GitHub status checks?

If so, do you know the general-purpose low-level feature that can be used to set UI in GitHub status checks?

mikeharder commented 1 year ago

If so, do you know the general-purpose low-level feature that can be used to set UI in GitHub status checks?

Maybe this API?

https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28#create-a-check-run--parameters

And I think we could create a GitHub Action to call the API. Could the GitHub action access the raw log of the TSV check, convert it to a nice markdown UI, and call the checks API to set the nice UI?

konrad-jamrozik commented 1 year ago

@mikeharder yes, here and here are the appendMsg sources.

For setting check status, conclusion and the various part of description, see this code I wrote recently and relevant types I added.

GitHub APIs used in the code above:

but this is only for writing. Unified pipeline never reads the check values, it only writes them.

When it comes to reading the check values the story is a bit more complicated. There are status checks (which have two subtypes: statuses and... checks), check runs and check suites. Doc. I had a call with @benbp where he explained to me some of the complexities. I can share the recording. I will be also diving into the problem head-first in the next 2 weeks to address these two work items:

konrad-jamrozik commented 1 year ago

@mikeharder re:

And I think we could create a GitHub Action to call the API. Could the GitHub action access the raw log of the TSV check, convert it to a nice markdown UI, and call the checks API to set the nice UI?

I believe so. The interesting question here is about the nice markdown UI. If you want to have a table looking exactly like in our unified pipeline-based checks, then the code for that is as I explained in my previous comment. That code is tightly coupled with the unified pipeline infrastructure which includes Cosmos DB instance, workflow that includes processing of events through event hub, a lot of custom types (like the Result) and bunch of other things.

My recommendation would be to write new, independent code to do the row data plumbing and table formatting, even if the table will end up looking a little bit differently. Doesn't matter IMO. We can use the existing unified pipeline code as an inspiration, but do not try to reuse it. At most: copy-paste-adapt-refactor.

It would just be cool if we try to write as much logic as possible in C# or TypeScript, not PowerShell. Keep the plumbing in PowerShell inside the GitHub action to absolute minimum we can get away with. I am sure we will end up with significant amount of code in the end, especially given we plan on migrating more things over from unified pipeline.

konrad-jamrozik commented 1 year ago

Here are some links I got from @benbp:

azure-sdk-actions/types.go at main · Azure/azure-sdk-actions (github.com) azure-sdk-actions/testpayloads at main · Azure/azure-sdk-actions (github.com) where I parse check suite url from PR json data: azure-sdk-actions/types.go at main · Azure/azure-sdk-actions (github.com) iterate check suites, get the IDs, call check runs. Each check suite json should have a url for its check runs that you can use

I am now working on getting the required checks info from GitHub in a way where I can also pull the TypeSpec Validation status into the Automated merging requirements met check, in addition to the statuses of the unified pipeline-based checks.

mikeharder commented 1 year ago

@mikeharder Mike Harder FTE re:

And I think we could create a GitHub Action to call the API. Could the GitHub action access the raw log of the TSV check, convert it to a nice markdown UI, and call the checks API to set the nice UI?

I believe so. The interesting question here is about the nice markdown UI. If you want to have a table looking exactly like in our unified pipeline-based checks, then the code for that is as I explained in my previous comment. That code is tightly coupled with the unified pipeline infrastructure which includes Cosmos DB instance, workflow that includes processing of events through event hub, a lot of custom types (like the Result) and bunch of other things.

My recommendation would be to write new, independent code to do the row data plumbing and table formatting, even if the table will end up looking a little bit differently. Doesn't matter IMO. We can use the existing unified pipeline code as an inspiration, but do not try to reuse it. At most: copy-paste-adapt-refactor.

It would just be cool if we try to write as much logic as possible in C# or TypeScript, not PowerShell. Keep the plumbing in PowerShell inside the GitHub action to absolute minimum we can get away with. I am sure we will end up with significant amount of code in the end, especially given we plan on migrating more things over from unified pipeline.

I agree with re-writing the component to convert from raw pipeline logs to nicely-formatted information directly in the PR. The format doesn't need to exactly match the previous checks, just be reasonable. Also agree with using TypeScript/JavaScript, since this seems like the de-facto language for GitHub Actions.

I'm still not sure if/how a GitHub action can read the raw pipeline logs, or if we'd need a bot to flow data from pipeline logs to GitHub Action (using an intermediate like EventHubs).

konrad-jamrozik commented 1 year ago

@mikeharder here is an example of multiple lines coming from ADO log to GitHub check view:

https://github.com/Azure/azure-rest-api-specs/pull/24548/checks?check_run_id=15419664559

image

mikeharder commented 1 year ago

@mikeharder Mike Harder FTE here is an example of multiple lines coming from ADO log to GitHub check view:

https://github.com/Azure/azure-rest-api-specs/pull/24548/checks?check_run_id=15419664559

image

I believe this update to the GitHub UI came from a bot calling the GitHub API, which we are trying to avoid.

I believe we can get some basic logging directly into the GitHub UI using the ##[error] log message.