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

Add nodejs access examples for REST APIs #602

Merged
merged 7 commits into from
Jul 30, 2023
Merged

Conversation

Sing-Li
Copy link
Contributor

@Sing-Li Sing-Li commented Jul 25, 2023

Entry point for mainstream node/JS/TS developers.

Add NodeJS streaming and non-streaming access examples for the REST APIs.

Based on the python examples from @sudeepag 🙏

@Hzfengsy
Copy link
Member

Great work! Unfortunately, I'm not familiar with JS/TS. perhaps @tqchen @junrushao if you could review?

@tqchen
Copy link
Contributor

tqchen commented Jul 26, 2023

@sudeepag can you review this?

@sudeepag
Copy link
Collaborator

sudeepag commented Jul 27, 2023

Nice work, @Sing-Li! A few suggestions:

  • Shall we move the python examples to mlc-llm/examples/rest/python and the nodejs examples can stay in mlc-llm/examples/rest/nodejs?
  • Let's also create a new directory at mlc-llm/examples/rest/resources with linux.txt and have both the python and nodejs examples reference the same file.
  • The sample_client and sample_openai examples work great for me, but with sample_langchain, I am getting the following error. Are we missing a dependency?
> [email protected] example
> ts-node --esm ./sample_langchain.ts

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /home/sudeepag/njs/mlc-llm/examples/rest/nodejs/sample_langchain.ts
    at new NodeError (node:internal/errors:405:5)
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:99:9)
    at defaultGetFormat (node:internal/modules/esm/get_format:142:36)
    at defaultLoad (node:internal/modules/esm/load:86:20)
    at nextLoad (node:internal/modules/esm/hooks:726:28)
    at load (/home/sudeepag/njs/mlc-llm/examples/rest/nodejs/node_modules/ts-node/dist/child/child-loader.js:19:122)
    at nextLoad (node:internal/modules/esm/hooks:726:28)
    at Hooks.load (node:internal/modules/esm/hooks:370:26)
    at MessagePort.handleMessage (node:internal/modules/esm/worker:168:24)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:762:20) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'

@Sing-Li
Copy link
Contributor Author

Sing-Li commented Jul 27, 2023

Nice work, @Sing-Li! A few suggestions:

Thank you @sudeepag ! And thanks for creating the rest API and python examples. I'm tracking your upstream Langchain pull request for changes in the embedding handling - (amazing!) , and hope to have those additional samples shortly after.

  • Shall we move the python examples to mlc-llm/examples/rest/python and the nodejs examples can stay in mlc-llm/examples/rest/nodejs?

Definitely! I'll make the changes.

  • Let's also create a new directory at mlc-llm/examples/rest/resources with linux.txt and have both the python and nodejs examples reference the same file.

Makes sense. I'll change and test the code accordingly.

  • The sample_client and sample_openai examples work great for me, but with sample_langchain, I am getting the following error. Are we missing a dependency?
> [email protected] example
> ts-node --esm ./sample_langchain.ts

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /home/sudeepag/njs/mlc-llm/examples/rest/nodejs/sample_langchain.ts
    at new NodeError (node:internal/errors:405:5)
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:99:9)
    at defaultGetFormat (node:internal/modules/esm/get_format:142:36)
    at defaultLoad (node:internal/modules/esm/load:86:20)
    at nextLoad (node:internal/modules/esm/hooks:726:28)
    at load (/home/sudeepag/njs/mlc-llm/examples/rest/nodejs/node_modules/ts-node/dist/child/child-loader.js:19:122)
    at nextLoad (node:internal/modules/esm/hooks:726:28)
    at Hooks.load (node:internal/modules/esm/hooks:370:26)
    at MessagePort.handleMessage (node:internal/modules/esm/worker:168:24)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:762:20) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'

Hmm. Could be, as I'm juggling Windows (where there is working vulkan driver) and Linux (WSL2 no vulkan but good git) environments. I'll check and adjust the PR accordingly.

Thanks again for the review.

@sudeepag
Copy link
Collaborator

@Sing-Li Thanks, my node version is v20.3.0 and npm version is 9.7.2, in case it helps. Happy to try other versions as well if it helps debug.

@Sing-Li
Copy link
Contributor Author

Sing-Li commented Jul 28, 2023

@sudeepag Thanks. You're right - it was indeed a node version problem! I've documented the required version v18.7.x in the README.

The PR is ready for review and testing. I have revised the directory structure as mentioned relocating the linux.txt to resources directory. Please re-test the python examples as well.

Thanks again.

@sudeepag
Copy link
Collaborator

@sudeepag Thanks. You're right - it was indeed a node version problem! I've documented the required version v18.7.x in the README.

@Sing-Li thanks for updating! just curious - why is there a v18 dependency? The Langchain readme says v20 should be supported. Is it an issue specific to langchain + typescript?

Copy link
Collaborator

@sudeepag sudeepag left a comment

Choose a reason for hiding this comment

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

Everything works great, thanks for adding the examples 👍

@junrushao junrushao merged commit 698d6c2 into mlc-ai:main Jul 30, 2023
@junrushao
Copy link
Member

Amazing work @Sing-Li!!

@Sing-Li
Copy link
Contributor Author

Sing-Li commented Aug 1, 2023

@sudeepag Thanks. You're right - it was indeed a node version problem! I've documented the required version v18.7.x in the README.

@Sing-Li thanks for updating! just curious - why is there a v18 dependency? The Langchain readme says v20 should be supported. Is it an issue specific to langchain + typescript?

@sudeepag Thanks again. Actually if it were 100% pure typescript code, directory, and project - v20 would work just fine.

It is because I insisted on making 2 of the sample files JS examples, and only 1 file TS (langchain) that is triggering the quirk. Plus the fact that I used some syntactic-sugar, good-for-example, code such as top level import (for example code, it is the easiest to understand and I wouldn't do it any other way).

This sort of mix-match config only happens in production for a few long-lived highly-evolved projects and as such is marginally supported by each of the new node versions --- it seems that v20 will eventually support this TypeStrong/ts-node#2033 (comment)

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.

5 participants