Migrate typespec validation #31033
Merged
Migrate typespec validation #31033
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Next Steps to Merge✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge. |
|
PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment. |
danieljurek
force-pushed
the
djurek/typespec-validation
branch
from
521aaa8
to
f7c31e3
Compare
danieljurek
force-pushed
the
djurek/typespec-validation
branch
from
8443eae
to
33006c5
Compare
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
requested changes
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
|
PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts. |
1 similar comment
|
PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts. |
danieljurek
force-pushed
the
djurek/typespec-validation
branch
from
229ea9b
to
17ba38a
Compare
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
* ` * ` * Revert "`" This reverts commit c26fc6a.
…sn't reach total-shards. Math is not allowed in expressions. This reverts commit 035bec7.
…t this time but adding back in because schema requires it.
danieljurek
force-pushed
the
djurek/typespec-validation
branch
from
7229bd3
to
a316274
Compare
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
Co-authored-by: Mike Harder <mharder@microsoft.com>
mikeharder
reviewed
Oct 25, 2024
mikeharder
reviewed
Oct 25, 2024
mikeharder
approved these changes
Oct 25, 2024
Co-authored-by: Mike Harder <mharder@microsoft.com>
weshaggard
reviewed
Oct 28, 2024
| fetch-depth: 2 | ||
|
|
||
| - name: Setup Node 20 and run `npm ci` | ||
| uses: ./.github/actions/setup-node-npm-ci |
There was a problem hiding this comment.
@mikeharder @danieljurek I thought github force these to be under the workflows directory. Is this a way we can make other reusable pieces for actions?
There was a problem hiding this comment.
The workflow itself must be under .github/workflows, but I believe custom actions can live anywhere in your repo, in another repo, etc.
We're still experimenting, but we're considering trying to provide most reusable code through custom actions, rather than using github-script everywhere and sharing JS code.
Custom actions can still use github-script internally, but the goal would be to make most of our leaf-node workflows as declarative as possible (no github-script).
weshaggard
reviewed
Oct 28, 2024
| } | ||
|
|
||
| if ($totalShards -gt $array.Length) { | ||
| throw "Cannot shard array into more pieces than there are elements" |
There was a problem hiding this comment.
Would we hit this case if there were only one spec discovered but we have it sharded in 3? I assume this wouldn't be the case for tsv-all but could be in other cases if we use this logic.
There was a problem hiding this comment.
Yes, currently the caller needs to check this first:
markcowl
pushed a commit
to markcowl/azure-rest-api-specs
that referenced
this pull request
Oct 29, 2024
* test breaking change mlc in js (Azure#3451) * ` * ` * Revert "`" This reverts commit c26fc6a. * Update specificationRepositoryConfiguration.json (Azure#3457) * Update specificationRepositoryConfiguration.json (Azure#3459) * Update specificationRepositoryConfiguration.json (Azure#3462) Revert python repo branch change as it was deleted. * Update specificationRepositoryConfiguration.json (Azure#3463) * Migrate typespec-validation.yml to GH Actions * path, fetch depth * Introduce error * Test a feature that AI says will collapse logs (doubtful it'll work) * Revert "Test a feature that AI says will collapse logs (doubtful it'll work)" This reverts commit 04aed83. * Log $LASTEXITCODE * Invalid syntax * Use error logging, remove continue-on-error because that is different behavior in GitHub Actions compared to ignoreLASTEXITCODE * Errors also get annotations in GitHub Actions * String replacement * Revert Logging-Functions.ps1 * Test composite action * File location * Invocation * Add typespec-validation-all.yml * Migrate typespec-validation-all.yml * Revert main.tsp invalid spec * fetch-depth: 2 * Long paths * checkout@v4 * Remove pipelines * *.yml -> *.yaml * Do not change specificationRepositoryConfiguration.json * Job names (for checks) * Add Skip/Take logic and matrix * Syntax * Rename * Remove "shell" * Try defaults.run.shell (unlikely to work) * bash * Name for actions/setup-node@v4 does not appear in the logs. Remove. * Use sharding semantics * Shard * Review feedback * Review feedback * +1 * Revert "Shard"... Display is confusing because it starts at 0 and doesn't reach total-shards. Math is not allowed in expressions. This reverts commit 035bec7. * Shard only if TotalShards > 0 * Remove description (schema validaiton does not pass, let's see what GH Actions says) * The `description` property is not absolutely required by GH Actions at this time but adding back in because schema requires it. * Add array functions and tests * Move Copy-ApiVersion.Tests.ps1 out of folder (adjust paths so they are accurate) * Add trailing newline * Add trailing newline * Update .github/workflows/typespec-validation.yaml Co-authored-by: Mike Harder <mharder@microsoft.com> * Review feedback * Update eng/scripts/TypeSpec-Validation.ps1 Co-authored-by: Mike Harder <mharder@microsoft.com> --------- Co-authored-by: Wanpeng Li <wanl@microsoft.com> Co-authored-by: Peng Jiahui <46921893+Alancere@users.noreply.github.com> Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com> Co-authored-by: Ray Chen <raychen@microsoft.com> Co-authored-by: Mike Harder <mharder@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes Azure/azure-sdk-tools#9175
Fixes #28974
Fixes #29635
Of note...
ignoreLASTEXITCODE: true... there's no direct analog in GitHub Actions (continue-on-error: truehides failures in these cases).Sharding and performance
For the
TypeSpec Validation - Allpipeline, this change shards using skip/take logic configured in the matrix. There is significant improvement at the cost of agent time.In its current configuration, the "last" job will scan all remaining folders after the existing 213 folders have been divided up. Folders that are "displaced" by the addition of a new folder earlier in the sequence can be scanned in later jobs and the "last" job's
take: 0means that all remaining entries will be scanned.If the "last" job starts taking long the matrix will have to be rebalanced to either add a shard or require other entries to take more folders.
Composite actions
This PR uses composite actions to do Node setup,
npm ci, andnpm ls -a... These are all collapsed into a single task in the logs. It's possible to further subdivide these for UI benefits if we care. We can use the composite actions in other places (like the eng-tools-test.yaml) but that's outside the scope of this change.Example failure:
PR with contrived failure: https://github.com/Azure/azure-rest-api-specs/actions/runs/11351864042/job/31573371561?pr=31034
When clicking on the failed check in the PR UI...
The user is taken straight to the logs: