Skip to content

Add step in build process to publish a tarfile #2077

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

Merged
merged 3 commits into from
May 30, 2024

Conversation

bkiu
Copy link
Contributor

@bkiu bkiu commented May 21, 2024

Description

In preparation for a feature to convert projects for offline in the pyscript-cli, adding a step to generate a tarball of the complied files available to for download.

Changes

Added a single step in the build process for generating a tarball.

Checklist

  • All tests pass locally # I'm unable to test it locally since it's the build process
  • I have updated CHANGELOG.md # Since it's just a step in the build process I haven't added anything to CHANGELOG.md
  • I have created documentation for this(if applicable)

@WebReflection
Copy link
Contributor

@bkiu would .zip work too? ... also, could you provide a minimal use case to reproduce and be sure we're aligned with the implementation and result? Thank you!

@bkiu
Copy link
Contributor Author

bkiu commented May 28, 2024

@WebReflection Right now it's a tarfile just so streamline the code here: pyscript/pyscript-cli#149

The other source is also given as a tarfile, so it allows for more code reuse to have it in a tarfile format. The use-case is also provided in that pull request of pulling down the files and putting them into the project. Let me know if more clarity is necessary beyond what is in the pull request for pyscript-cli already.

@WebReflection
Copy link
Contributor

@bkiu this line in particular won't make it:

https://github.com/pyscript/pyscript-cli/pull/149/files#diff-e5766bc775b761da633caf3a4bb8dba37023302062a9a4bf95087bdf74a2dfd2R171

we can't block URL requests on the browser ... would an async resolution work too?

@bkiu
Copy link
Contributor Author

bkiu commented May 28, 2024

@WebReflection Hmm, so that line was actually there before I worked on the code. Since this is in the pyscript-cli repo, it's not something (as far as I'm aware) that's going to be run in the browser, but only by somebody executing it in their terminal. There's a few areas that downloading is required using requests.get like this. It seems like blocking would be expected behavior with a command line tool for downloading assets.

@WebReflection
Copy link
Contributor

@bkiu you are right, I did by mistake thought this was a FE matter as this repo is for the FE side of affairs.

I think at this point I need to understand what's going on here though ... PyScript can run offline but it requires also an interpreter with its dist files (JS module + WASM) and, optionally, its dependencies ... is this PR enough to guarantee all that will be eventually also available within the tar file?

If yes is the answer, I am OK in merging this.

@bkiu
Copy link
Contributor Author

bkiu commented May 29, 2024

@WebReflection I worked with @FabioRosado during PyCon sprints to get the details working, and I was able to automate downloading the necessary files and, optionally, the full set of pyodide modules as well. Once this is merged, the functionality can be tested in my other PR in pyscript-cli.

@FabioRosado
Copy link
Contributor

@bkiu this line in particular won't make it:

https://github.com/pyscript/pyscript-cli/pull/149/files#diff-e5766bc775b761da633caf3a4bb8dba37023302062a9a4bf95087bdf74a2dfd2R171

we can't block URL requests on the browser ... would an async resolution work too?

For context this code provided is for the CLI which is run in the user's terminal so we don't need to worry about blocking calls like we do in the pyscript. But following the rest of the convo I think you got that haha

Seems like this particular PR is simply adding the release tar as a step and then it will be uploaded to the bucket for the released version. I don't think there is any issues with it 🤔

@fpliger any thoughts since you worked with @bkiu in PyCon?

@WebReflection
Copy link
Contributor

FYI the full set of pyodide modules is a 280MB and counting blob ... I am not sure this actually packages only required modules instead but if it doesn't, we should be aware that tarball gonna hit hard on bandwidth

@bkiu
Copy link
Contributor Author

bkiu commented May 30, 2024

@WebReflection Per this line the download_full_pyodide is a boolean flag that is off by default, since the download is quite massive.

@WebReflection WebReflection merged commit 320a537 into pyscript:main May 30, 2024
@fpliger
Copy link
Contributor

fpliger commented May 30, 2024

Thank you @bkiu !!

@WebReflection @FabioRosado we don't need to put the tar in the same place we put everything else. This PR is a starting point, we can improve if we find a better place and worflow.

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.

4 participants