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.

Sign up for GitHub

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

Jump to bottom

wrap runPython in async #1212

Merged
merged 16 commits into from Feb 21, 2023
Merged

wrap runPython in async #1212

merged 16 commits into from Feb 21, 2023

Conversation

madhur-tandon
Copy link
Member

CC: @JeffersGlass @antocuni @hoodmane

TODO comments still required at two places.

Thing to consider: In pyrepl.ts -- should the call to pyExec be inside a mutex lock? Just the way it was done for the pyExec call in pyscript.ts

@madhur-tandon madhur-tandon added the sprint issue has been pulled into current sprint and is actively being worked on label Feb 21, 2023
antocuni
antocuni reviewed Feb 21, 2023
View reviewed changes
Copy link
Contributor

@antocuni antocuni left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

should the call to pyExec be inside a mutex lock? Just the way it was done for the pyExec call in pyscript.ts

I think it is not needed

pyscriptjs/src/pyodide.ts Show resolved Hide resolved
pyscriptjs/src/main.ts Outdated Show resolved Hide resolved
pyscriptjs/tests/integration/test_01_basic.py Outdated Show resolved Hide resolved
hoodmane
hoodmane reviewed Feb 21, 2023
View reviewed changes
pyscriptjs/src/main.ts Outdated Show resolved Hide resolved
@madhur-tandon madhur-tandon merged commit e2c2459 into main Feb 21, 2023
@madhur-tandon madhur-tandon deleted the antocuni/async-interpreter-run branch February 21, 2023 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoodmane hoodmane hoodmane left review comments

@antocuni antocuni antocuni approved these changes

Assignees
No one assigned
Labels
sprint issue has been pulled into current sprint and is actively being worked on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@madhur-tandon @antocuni @hoodmane @JeffersGlass