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

Add Support for Changed Spec Identification in spec-gen-sdk-runner tool #32763

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions eng/pipelines/templates/stages/archetype-spec-gen-sdk.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
parameters:

Check failure on line 1 in eng/pipelines/templates/stages/archetype-spec-gen-sdk.yml

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/pipelines/templates/stages/archetype-spec-gen-sdk.yml' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.
- name: SpecRepoUrl
type: string
- name: SdkRepoUrl
Expand Down Expand Up @@ -27,11 +27,13 @@
extends:
template: /eng/pipelines/templates/stages/1es-redirect.yml
parameters:
Use1ESOfficial: false
stages:
- stage: Build
displayName: 'SDK Generation'
jobs:
- job:
timeoutInMinutes: 2400

variables:
- template: /eng/pipelines/templates/variables/image.yml
Expand All @@ -56,12 +58,12 @@
displayName: Publish SDK artifacts to Pipeline Artifacts
condition: and(ne(variables['ValidationResult'], ''), eq(variables['HasSDKArtifact'], 'true'))
artifactName: $(sdkArtifactName)
targetPath: "$(System.DefaultWorkingDirectory)/out/generatedSdkArtifacts"
targetPath: "$(System.DefaultWorkingDirectory)/out/stagedArtifacts"
- output: pipelineArtifact
displayName: Publish API View artifacts to Pipeline Artifacts
condition: and(ne(variables['ValidationResult'], ''), eq(variables['HasApiViewArtifact'], 'true'))
artifactName: $(ArtifactName)
targetPath: "$(System.DefaultWorkingDirectory)/out/sdkApiViewArtifacts"
targetPath: "$(System.DefaultWorkingDirectory)/out/stagedArtifacts"
- output: pipelineArtifact
displayName: Publish logs to Pipeline Artifacts
condition: ne(variables['ValidationResult'], '')
Expand Down Expand Up @@ -168,7 +170,7 @@
optional_params=""
sdk_gen_info="sdk generation from Config : "

if [ "${{ parameters.ConfigType }}" = "TypeSpec" ]; then
if [ "$(Build.Reason)" != "PullRequest" ] && [ "${{ parameters.ConfigType }}" = "TypeSpec" ]; then
optional_params="$optional_params --tsp-config-relative-path ${{ parameters.ConfigPath }}"
sdk_gen_info="$sdk_gen_info '${{ parameters.ConfigPath }}',"
elif [ "${{ parameters.ConfigType }}" = "OpenAPI" ]; then
Expand All @@ -177,7 +179,7 @@
fi

if [ "$(Build.Reason)" = "PullRequest" ]; then
optional_params="$optional_params --pr-number=$(System.PullRequest.PullRequestNumber)"
optional_params="$optional_params --pr-number $(System.PullRequest.PullRequestNumber)"
specPrUrl="${{ parameters.SpecRepoUrl }}/pull/$(System.PullRequest.PullRequestNumber)"
sdk_gen_info="$sdk_gen_info spec PR: $specPrUrl"
fi
Expand Down
Empty file modified eng/tools/spec-gen-sdk-runner/cmd/spec-gen-sdk-runner.js
100644 → 100755
Empty file.
5 changes: 5 additions & 0 deletions eng/tools/spec-gen-sdk-runner/eslint.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// @ts-check

Check failure on line 1 in eng/tools/spec-gen-sdk-runner/eslint.config.js

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/tools/spec-gen-sdk-runner/eslint.config.js' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.

// The overall contents of this file is based on:
// https://typescript-eslint.io/getting-started#step-2-configuration
Expand Down Expand Up @@ -68,11 +68,16 @@
"@typescript-eslint/restrict-template-expressions": "off",
"@typescript-eslint/no-unsafe-assignment": "off",
"@typescript-eslint/no-unsafe-argument": "off",
"@typescript-eslint/consistent-indexed-object-style": "off",
"@typescript-eslint/no-unnecessary-condition": "off",
"@typescript-eslint/consistent-type-definitions": "off",
"@typescript-eslint/no-inferrable-types": "off",

// We want more flexibility with file names.
// https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/filename-case.md
"unicorn/filename-case": "off",
"unicorn/prefer-ternary": "off",
"unicorn/no-useless-undefined": "off",

// We prefer to have explicitly import at the top of the file, even if the same element is exported again,
// which we do in index.ts files.
Expand Down
96 changes: 96 additions & 0 deletions eng/tools/spec-gen-sdk-runner/src/change-files.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import path from "node:path";

Check failure on line 1 in eng/tools/spec-gen-sdk-runner/src/change-files.ts

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/tools/spec-gen-sdk-runner/src/change-files.ts' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.
import {
execAsync,
getChangedFiles,
searchRelatedParentFolders,
searchRelatedTypeSpecProjectBySharedLibrary,
searchSharedLibrary,
} from "./utils.js";
import { logMessage } from "./log.js";
import { SpecGenSdkCmdInput } from "./types.js";

const readmeMdRegex = /^readme.md$/;
const typespecProjectRegex = /^tspconfig.yaml$/;
const typespecProjectSharedLibraryRegex = /[^/]+\.Shared/;

type ChangedSpecs = {
[K in "readmeMd" | "typespecProject"]?: string;
} & {
specs: string[];
};

export async function detectChangedSpecConfigFiles(
commandInput: SpecGenSdkCmdInput,
): Promise<ChangedSpecs[]> {
const prChangedFiles: string[] = getChangedFiles(commandInput.localSpecRepoPath) ?? [];
if (prChangedFiles.length === 0) {
logMessage("No files changed in the PR");
}
logMessage(`Changed files in the PR: ${prChangedFiles.length}`);
const fileList = prChangedFiles.filter((p) => !p.includes("/scenarios/"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is special about "scenarios" here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We intend to exclude SDK generation for changes coming from the scenarios folder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also other paths like examples should those be included? I'm mostly asking this question as it seems a little odd to only figure out this one folder type if what you are looking for is json files.

Copy link
Member Author

@raych1 raych1 Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some other file types also need to be included, such as *.tsp, tspconfig.yaml, readme.md.
Changes in the examples folder would need to trigger the automation run because samples would be used in the SDK test/sample generation step. I am not aware of any other folders we might have in the specification folder. I will also add 'specification' to the filter.

const { stdout: headCommitRaw } = await execAsync("git rev-parse HEAD");
const headCommit = headCommitRaw.trim(); // Trim any newline characters
const { stdout: treeIdRaw } = await execAsync(`git rev-parse ${headCommit}^{tree}`);
const treeId = treeIdRaw.trim();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we looking at git here? We should just be using the files on disk and assume that repo is correctly checked out.


logMessage(`Related readme.md and typespec project list:`);
const changedSpecs: ChangedSpecs[] = [];
const readmeMDResult = await searchRelatedParentFolders(fileList, {
searchFileRegex: readmeMdRegex,
specFolder: commandInput.localSpecRepoPath,
treeId,
});
const typespecProjectResult = await searchRelatedParentFolders(fileList, {
searchFileRegex: typespecProjectRegex,
specFolder: commandInput.localSpecRepoPath,
treeId,
});
const typespecProjectSharedLibraries = searchSharedLibrary(fileList, {
searchFileRegex: typespecProjectSharedLibraryRegex,
specFolder: commandInput.localSpecRepoPath,
treeId,
});
const typespecProjectResultSearchedBySharedLibrary =
await searchRelatedTypeSpecProjectBySharedLibrary(typespecProjectSharedLibraries, {
searchFileRegex: typespecProjectRegex,
specFolder: commandInput.localSpecRepoPath,
treeId,
});
for (const folderPath of Object.keys(typespecProjectResultSearchedBySharedLibrary)) {
if (typespecProjectResult[folderPath]) {
typespecProjectResult[folderPath] = [
...typespecProjectResult[folderPath],
...typespecProjectResultSearchedBySharedLibrary[folderPath],
];
} else {
typespecProjectResult[folderPath] = typespecProjectResultSearchedBySharedLibrary[folderPath];
}
}
const result: { [folderPath: string]: string[] } = {};
for (const folderPath of Object.keys(readmeMDResult)) {
result[folderPath] = readmeMDResult[folderPath];
}

for (const folderPath of Object.keys(typespecProjectResult)) {
result[folderPath] = typespecProjectResult[folderPath];
}
for (const folderPath of Object.keys(result)) {
const readmeMdPath = path.join(folderPath, "readme.md");
const cs: ChangedSpecs = {
readmeMd: readmeMdPath,
specs: readmeMDResult[folderPath],
};

if (typespecProjectResult[folderPath]) {
delete cs.readmeMd;
cs.specs = typespecProjectResult[folderPath];
cs.typespecProject = path.join(folderPath, "tspconfig.yaml");
logMessage(`\t tspconfig.yaml file: ${cs.typespecProject}`);
} else {
logMessage(`\t readme.md file: ${readmeMdPath}`);
}
changedSpecs.push(cs);
}

return changedSpecs;
}
93 changes: 81 additions & 12 deletions eng/tools/spec-gen-sdk-runner/src/commands.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fs from "node:fs";

Check failure on line 1 in eng/tools/spec-gen-sdk-runner/src/commands.ts

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/tools/spec-gen-sdk-runner/src/commands.ts' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.
import path from "node:path";
import { fileURLToPath } from "node:url";
import {
Expand All @@ -6,9 +6,11 @@
getArgumentValue,
runSpecGenSdkCommand,
getAllTypeSpecPaths,
resetGitRepo,
} from "./utils.js";
import { LogLevel, logMessage, vsoAddAttachment } from "./log.js";
import { SpecGenSdkCmdInput } from "./types.js";
import { detectChangedSpecConfigFiles } from "./change-files.js";

export async function generateSdkForSingleSpec(): Promise<number> {
// Parse the arguments
Expand Down Expand Up @@ -44,8 +46,71 @@
return statusCode;
}

/* Generate SDKs for spec pull request */
export async function generateSdkForSpecPr(): Promise<number> {
// Parse the arguments
const commandInput: SpecGenSdkCmdInput = parseArguments();
// Construct the spec-gen-sdk command
const specGenSdkCommand = prepareSpecGenSdkCommand(commandInput);
// Get the spec paths from the changed files
const changedSpecs = await detectChangedSpecConfigFiles(commandInput);

let statusCode = 0;
let pushedSpecConfigCount;
for (const changedSpec of changedSpecs) {
if (!changedSpec.typespecProject && !changedSpec.readmeMd) {
logMessage("No spec config file found in the changed files", LogLevel.Warn);
continue;
}
pushedSpecConfigCount = 0;
if (changedSpec.typespecProject) {
specGenSdkCommand.push("--tsp-config-relative-path", changedSpec.typespecProject);
pushedSpecConfigCount++;
}
if (changedSpec.readmeMd) {
specGenSdkCommand.push("--readme-relative-path", changedSpec.readmeMd);
pushedSpecConfigCount++;
}
const changedSpecPath = changedSpec.typespecProject ?? changedSpec.readmeMd;
logMessage(`Generating SDK from ${changedSpecPath}`, LogLevel.Group);
logMessage(`Command:${specGenSdkCommand.join(" ")}`);

try {
await resetGitRepo(commandInput.localSdkRepoPath);
await runSpecGenSdkCommand(specGenSdkCommand);
logMessage("Command executed successfully");
} catch (error) {
logMessage(`Error executing command:${error}`, LogLevel.Error);
statusCode = 1;
}
// Pop the spec config path from specGenSdkCommand
for (let index = 0; index < pushedSpecConfigCount * 2; index++) {
specGenSdkCommand.pop();
}
// Read the execution report to determine if the generation was successful
const executionReportPath = path.join(
commandInput.workingFolder,
`${commandInput.sdkRepoName}_tmp/execution-report.json`,
);
try {
const executionReport = JSON.parse(fs.readFileSync(executionReportPath, "utf8"));
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const executionResult = executionReport.executionResult;
logMessage(`Execution Result:${executionResult}`);
} catch (error) {
logMessage(
`Error reading execution report at ${executionReportPath}:${error}`,
LogLevel.Error,
);
statusCode = 1;
}
logMessage("ending group logging", LogLevel.EndGroup);
}
return statusCode;
}

/**
* Generate SDKs for all specs.
* Generate SDKs for batch specs.
*/
export async function generateSdkForBatchSpecs(runMode: string): Promise<number> {
// Parse the arguments
Expand All @@ -60,9 +125,9 @@
let markdownContent = "\n";
let failedContent = `## Spec Failures in the Generation Process\n`;
let succeededContent = `## Successful Specs in the Generation Process\n`;
let undefinedContent = `## Disabled Specs in the Generation Process\n`;
let notEnabledContent = `## Specs with SDK Not Enabled\n`;
let failedCount = 0;
let undefinedCount = 0;
let notEnabledCount = 0;
let succeededCount = 0;

// Generate SDKs for each spec
Expand All @@ -75,6 +140,7 @@
}
logMessage(`Command:${specGenSdkCommand.join(" ")}`);
try {
await resetGitRepo(commandInput.localSdkRepoPath);
await runSpecGenSdkCommand(specGenSdkCommand);
logMessage("Command executed successfully");
} catch (error) {
Expand All @@ -96,12 +162,12 @@
const executionResult = executionReport.executionResult;
logMessage(`Execution Result:${executionResult}`);

if (executionResult === "succeeded") {
if (executionResult === "succeeded" || executionResult === "warning") {
succeededContent += `${specConfigPath},`;
succeededCount++;
} else if (executionResult === undefined) {
undefinedContent += `${specConfigPath},`;
undefinedCount++;
} else if (executionResult === "notEnabled") {
notEnabledContent += `${specConfigPath},`;
notEnabledCount++;
} else {
failedContent += `${specConfigPath},`;
failedCount++;
Expand All @@ -118,15 +184,15 @@
if (failedCount > 0) {
markdownContent += `${failedContent}\n`;
}
if (undefinedCount > 0) {
markdownContent += `${undefinedContent}\n`;
if (notEnabledCount > 0) {
markdownContent += `${notEnabledContent}\n`;
}
if (succeededCount > 0) {
markdownContent += `${succeededContent}\n`;
}
markdownContent += failedCount ? `## Total Failed Specs\n ${failedCount}\n` : "";
markdownContent += undefinedCount
? `## Total Disabled Specs in the Configuration\n ${undefinedCount}\n`
markdownContent += notEnabledCount
? `## Total Specs with SDK not enabled in the Configuration\n ${notEnabledCount}\n`
: "";
markdownContent += succeededCount ? `## Total Successful Specs\n ${succeededCount}\n` : "";
markdownContent += `## Total Specs Count\n ${specConfigPaths.length}\n\n`;
Expand Down Expand Up @@ -251,7 +317,10 @@
break;
}
case "sample-typespecs": {
specConfigPaths.push("specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml");
specConfigPaths.push(
"specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml",
"specification/contosowidgetmanager/Contoso.WidgetManager/tspconfig.yaml",
);
}
}
return specConfigPaths;
Expand Down
11 changes: 10 additions & 1 deletion eng/tools/spec-gen-sdk-runner/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
import { exit } from "node:process";

Check failure on line 1 in eng/tools/spec-gen-sdk-runner/src/index.ts

View workflow job for this annotation

GitHub Actions / Protected Files

File 'eng/tools/spec-gen-sdk-runner/src/index.ts' should only be updated by the Azure SDK team. If intentional, the PR may be merged by the Azure SDK team via bypassing the branch protections.
import { getArgumentValue } from "./utils.js";
import { generateSdkForBatchSpecs, generateSdkForSingleSpec } from "./commands.js";
import {
generateSdkForBatchSpecs,
generateSdkForSingleSpec,
generateSdkForSpecPr,
} from "./commands.js";

export async function main() {
// Get the arguments passed to the script
const args: string[] = process.argv.slice(2);
// Log the arguments to the console
console.log("Arguments passed to the script:", args.join(" "));
const runMode: string = getArgumentValue(args, "--rm", "");
const pullRequestNumber: string = getArgumentValue(args, "--pr-number", "");
let statusCode = 0;
if (runMode) {
statusCode = await generateSdkForBatchSpecs(runMode);
} else if (pullRequestNumber) {
statusCode = await generateSdkForSpecPr();
} else {
statusCode = await generateSdkForSingleSpec();
}
Expand Down
Loading
Loading