Skip to content

Support generating Python SDK with Pyodide#4784

Closed
YalinLi0312 wants to merge 55 commits into
microsoft:mainfrom
YalinLi0312:pyodide
Closed

Support generating Python SDK with Pyodide#4784
YalinLi0312 wants to merge 55 commits into
microsoft:mainfrom
YalinLi0312:pyodide

Conversation

@YalinLi0312

Copy link
Copy Markdown

No description provided.

@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:python Issue for the Python client emitter: @typespec/http-client-python label Oct 17, 2024
@azure-sdk

Copy link
Copy Markdown
Collaborator

No changes needing a change description found.

@azure-sdk

Copy link
Copy Markdown
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

@YalinLi0312 YalinLi0312 requested a review from lmazuel October 17, 2024 22:44
Comment thread packages/http-client-python/package.json

@iscai-msft iscai-msft left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looking really good, thanks @YalinLi0312 !

Comment thread packages/http-client-python/eng/scripts/Test-Packages.ps1 Outdated
Comment thread packages/http-client-python/eng/scripts/ci/regenerate.ts
Comment thread packages/http-client-python/package.json Outdated
if (sdkContext.arm === true) {
commandArgs.push("--azure-arm=true");
}
if (resolvedOptions.flavor === "azure") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The logic about options (e.g. "azure-arm"/"flavor") between python mode and pyodide mode is same so it is better to handle them in one central place.

Write-Host "Building project ..."
& npm run build

Write-Host "Regenerating project with Pyodide ..."

@msyyc msyyc Nov 20, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there is no need to add new step in eng to run test twice with python and pyodide mode to make the CI time double than now. We can run azure test with python mode and unbranded test with pyodide mode. Then you just need to add use-pyodide: true here

await regenerate({ ...flags, flavor: "unbranded" });
.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we just want to make sure that generating with pyodide and with python result in the same output, so I think it is necessary to run twice

@tadelesh tadelesh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

some nit

resolvedOptions["package-pprint-name"] = `"${resolvedOptions["package-pprint-name"]}"`;
}
if (resolvedOptions["use-pyodide"]) {
const commandArgs: Record<string, string> = {};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the command args should be same whatever use pyodide or not, it's better to consolidate the logic.

Comment on lines +131 to +142
async def main():
import warnings
with warnings.catch_warnings():
warnings.simplefilter("ignore", SyntaxWarning)
from pygen import m2r, preprocess, codegen, black

m2r.M2R(output_folder=outputFolder, cadl_file=yamlRelativePath, **commandArgs).process()
preprocess.PreProcessPlugin(output_folder=outputFolder, cadl_file=yamlRelativePath, **commandArgs).process()
codegen.CodeGenerator(output_folder=outputFolder, cadl_file=yamlRelativePath, **commandArgs).process()
black.BlackScriptPlugin(output_folder=outputFolder, **commandArgs).process()

await main()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is pyodide able to run a py file? i just wondering if two paths could be consolidated. previously, we used py file. here, we use inline code.


await main()
`;
await pyodide.runPythonAsync(python, { globals });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how will pyodide deal with exception?

Comment on lines +153 to +154
await runPython3("./eng/scripts/setup/install.py");
await runPython3("./eng/scripts/setup/prepare.py");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i'm wondering how the exception be caught here?

@iscai-msft

Copy link
Copy Markdown
Member

closing in favor of #5289

@iscai-msft iscai-msft closed this Dec 6, 2024
@iscai-msft iscai-msft reopened this Dec 11, 2024
@tadelesh tadelesh closed this Dec 18, 2024
github-merge-queue Bot pushed a commit that referenced this pull request Dec 25, 2024
Mostly copied from #4784,
thank you @YalinLi0312

---------

Co-authored-by: iscai-msft <isabellavcai@gmail.com>
Co-authored-by: tadelesh <tadelesh.shi@live.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:python Issue for the Python client emitter: @typespec/http-client-python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants