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

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

Open
mikeharder opened this issue Jul 26, 2023 · 9 comments
Open

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

mikeharder opened this issue Jul 26, 2023 · 9 comments
Assignees
Labels
Spec PR Tools Tooling that runs in azure-rest-api-specs repo.

Comments

@mikeharder
Copy link
Member

mikeharder commented Jul 26, 2023

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:

if ($pathsWithErrors.Count -gt 0)

@konrad-jamrozik
Copy link

konrad-jamrozik commented Aug 9, 2023

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
Copy link
Member Author

@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
Copy link
Member Author

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
Copy link

konrad-jamrozik commented Aug 9, 2023

@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
Copy link

konrad-jamrozik commented Aug 9, 2023

@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
Copy link

konrad-jamrozik commented Aug 9, 2023

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
Copy link
Member Author

@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).

@mikeharder mikeharder moved this from 🐝 Dev to 📋 Backlog in Azure SDK EngSys 🤖🧠 Aug 10, 2023
@konrad-jamrozik
Copy link

@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
Copy link
Member Author

@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.

@mikeharder mikeharder assigned mikeharder and unassigned ckairen Aug 22, 2023
@mikeharder mikeharder moved this from 📋 Backlog to 🐝 Dev in Azure SDK EngSys 🤖🧠 Aug 22, 2023
@mikeharder mikeharder moved this from 🐝 Dev to 📋 Backlog in Azure SDK EngSys 🤖🧠 Aug 24, 2023
@mikeharder mikeharder assigned ckairen and unassigned mikeharder Sep 21, 2023
@mikeharder mikeharder moved this from 📋 Backlog to 🐝 Dev in Azure SDK EngSys 🤖🧠 Sep 21, 2023
@mikeharder mikeharder assigned mikeharder and unassigned ckairen Sep 21, 2023
@mikeharder mikeharder moved this from 🐝 Dev to 📋 Backlog in Azure SDK EngSys 🤖🧠 Sep 21, 2023
@mikeharder mikeharder assigned ckairen and unassigned mikeharder Sep 27, 2023
@konrad-jamrozik konrad-jamrozik added the Spec PR Tools Tooling that runs in azure-rest-api-specs repo. label Nov 3, 2023
@mikeharder mikeharder moved this from 📋 Backlog to 🐝 Dev in Azure SDK EngSys 🤖🧠 Jan 25, 2024
@ckairen ckairen assigned mikeharder and unassigned ckairen May 1, 2024
@mikeharder mikeharder moved this from 🐝 Dev to 📋 Backlog in Azure SDK EngSys 🤖🧠 May 2, 2024
@mikeharder mikeharder moved this from 📋 Backlog to 🐝 Dev in Azure SDK EngSys 🤖🧠 Jun 27, 2024
@mikeharder mikeharder moved this from 🐝 Dev to 📋 Backlog in Azure SDK EngSys 🤖🧠 Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Spec PR Tools Tooling that runs in azure-rest-api-specs repo.
Projects
Status: 📋 Backlog
Status: No status
Development

No branches or pull requests

3 participants