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

Pyodide headers #1104

Merged
merged 9 commits into from
Oct 17, 2024
Merged

Pyodide headers #1104

merged 9 commits into from
Oct 17, 2024

Conversation

sra405
Copy link
Contributor

@sra405 sra405 commented Oct 15, 2024

@sra405 sra405 temporarily deployed to previews/1104/merge October 15, 2024 09:01 — with GitHub Actions Inactive
@sra405 sra405 temporarily deployed to previews/1104/merge October 15, 2024 16:18 — with GitHub Actions Inactive
@sra405 sra405 temporarily deployed to previews/1104/merge October 16, 2024 08:24 — with GitHub Actions Inactive
@sra405 sra405 temporarily deployed to previews/1104/merge October 16, 2024 08:45 — with GitHub Actions Inactive
@sra405 sra405 temporarily deployed to previews/1104/merge October 16, 2024 08:51 — with GitHub Actions Inactive
Copy link
Contributor

@loiswells97 loiswells97 left a comment

Choose a reason for hiding this comment

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

Just a few questions, but otherwise woohoo!

Comment on lines -167 to +169
`${process.env.ASSETS_URL}/packages/turtle-0.0.1-py3-none-any.whl`,
`${process.env.ASSETS_URL}/pyodide/packages/turtle-0.0.1-py3-none-any.whl`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this one ASSETS_URL when the ones above are PUBLIC_URL? I've gotten a bit confused about what the difference is between the two vars to be honest with you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah they can both probably be merged into one env var now. When the site was deployed and assets were in the bucket but on the same domain it was causing a problem so I exposed the bucket via a URL and hit that directly a while back. But now its all bucket based

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing them all to ASSETS_URL in this file then we can remove that later if we want to 👍

Comment on lines 137 to 138
// TODO: Sk.sense_hat.mz_criteria.noInputEvents = false;

Copy link
Contributor

Choose a reason for hiding this comment

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

Was this comment deleted on purpose or has it been slashed by Copilot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copilot 🤦 will restore

Comment on lines 160 to 154
stdinClosed.current = true; // Don't accept any more stdin this run.
stdinClosed.current = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as my other question about deleted comment above ⤴️

@@ -218,20 +212,19 @@ const PyodideRunner = ({ active }) => {
writeFile([name, extension].join("."), content);
}

// program is the content of the component with name main and extension py
Copy link
Contributor

Choose a reason for hiding this comment

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

Another deleted comment

Comment on lines 219 to 227
if (interruptBuffer.current) {
interruptBuffer.current[0] = 0; // Clear previous signals.
interruptBuffer.current[0] = 0;
}
pyodideWorker.postMessage({ method: "runPython", python: program });
};

const handleStop = () => {
if (interruptBuffer.current) {
interruptBuffer.current[0] = 2; // Send a SIGINT signal.
interruptBuffer.current[0] = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

More comments

Comment on lines +98 to +100
// Pyodide - required for input and code interruption - needed on the host app
"Cross-Origin-Opener-Policy": "same-origin",
"Cross-Origin-Embedder-Policy": "require-corp",
Copy link
Contributor

Choose a reason for hiding this comment

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

So are we just assuming that host pages will have to add these themselves for now, or am I not understanding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea that's right. So on editor.raspberrypi.org we've already got these set 👍

Copy link
Contributor

@loiswells97 loiswells97 left a comment

Choose a reason for hiding this comment

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

WOOOOHOOOOO 🥳 🎉 🚢 🚀

@sra405 sra405 merged commit 363a8e3 into pyodide-migration Oct 17, 2024
8 checks passed
@sra405 sra405 deleted the pyodide-headers-webpack-4 branch October 17, 2024 15:36
@loiswells97 loiswells97 mentioned this pull request Oct 22, 2024
loiswells97 added a commit that referenced this pull request Oct 22, 2024
## [0.28.0] - 2024-10-22 

### Added

- PyodideWorker setup for the editor (#1104)
- Enabling `pyodide` support in the web component (#1090)
- `Pyodide` `matplotlib` support (#1087)
- Tests for running simple programs in `pyodide` and `skulpt` (#1100)
- Fall back to `skulpt` if the host is not `crossOriginIsolated` (#1107)
- `Pyodide` `seaborn` support (#1106)
- `Pyodide` module caching (#1113)

### Changed

- Upgrade to `webpack 5` (#1096)
- Bump `pyodide` to `v0.26.2` (#1098)
- Updated the ImportErrors message (#1105)
- In ErrorMessage component added the way to display html elements in
string (#1105)

### Fixed

- Dynamic runner switching with more than one `python` file (#1097)
- Pyodide running the correct file (`main.py`) when there are multiple
`python` files (#1097)
- Build to include public files (#1112)
- Persisting choice of tabbed/split view when running `python` code
(#1114)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants