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

Replace parquetjs for better deno compatibility? #102

Open
canadaduane opened this issue Nov 28, 2024 · 0 comments · May be fixed by #105
Open

Replace parquetjs for better deno compatibility? #102

canadaduane opened this issue Nov 28, 2024 · 0 comments · May be fixed by #105

Comments

@canadaduane
Copy link

canadaduane commented Nov 28, 2024

Currently, when adding npm:together-ai to a deno project, the following warnings are shown:

$ deno add npm:together-ai
Add npm:[email protected]
Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
┠─ npm:[email protected]
┃
┠─ This may cause the packages to not work correctly.
┠─ Lifecycle scripts are only supported when using a `node_modules` directory.
┠─ Enable it in your deno config file:
┖─ "nodeModulesDir": "auto"
Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:
┠─ npm:[email protected]
┃
┠─ This may cause the packages to not work correctly.
┠─ Lifecycle scripts are only supported when using a `node_modules` directory.
┠─ Enable it in your deno config file:
┖─ "nodeModulesDir": "auto"

As far as I can tell, lzo is unnecessary, and exists only because it is a transitive dependency of parquetjs, which hasn't been maintained in 5 years or so. It uses a node-gyp build pipeline to add a few C functions for fast LZO compression/decompression.

Possible fixes:

  • replace parquetjs with @dsnp/parquetjs: this package seems to be maintained and appears to cover the needs of together-ai afaict. Note that they explicitly removed lzo some time ago due to the compress/decompress tests not passing, which may also be true of parquetjs: LZO and LZO_RAW Support LibertyDSNP/parquetjs#18
  • replace parquetjs with parquetjs-lite: this package does not look maintained, like parquetjs, but it does not have a dependency on lzo.

The only functionality that might depend on lzo is the upload function for uploading fine tunes. Some testing might be needed to confirm that it works as expected, once one of the above two replacements is made.

Fixing this would make users of together-ai in the deno ecosystem happy and would offer confidence that "everything works fine" rather than a few scare warnings.

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 a pull request may close this issue.

1 participant