proxy openai requests from js library #4 #10
Conversation
src/promptlayer/index.ts
Outdated
| const response = Reflect.apply(target, thisArg, argArray); | ||
| if (response instanceof Promise) { | ||
| response.then((request_response) => { | ||
| const request_end_time = new Date().toISOString(); |
There was a problem hiding this comment.
Didn't we need to do something special for this in JS?
There was a problem hiding this comment.
Are we fixing that on the backend? So many users had that issue
There was a problem hiding this comment.
What issue are we talking about exactly?
There was a problem hiding this comment.
People have to do this currently when using Javascript
There was a problem hiding this comment.
@jzone3 this should not have any issues according to the backend.
src/promptlayer/index.ts
Outdated
| const newTarget = Reflect.construct(target, args); | ||
| Object.defineProperties(newTarget, { | ||
| function_name: { | ||
| value: args[1] || "openai", |
There was a problem hiding this comment.
@jzone3 still need to figure out this as well. This is likely to change given how I figure out lazy loading to work. For now consider this is just there to represent a default value. In future I expect it to be passed as a function parameter that assigned default function name just like how we do it in python.
There was a problem hiding this comment.
Nice, makes sense. We can even be more explicit "unknown_js_function" if I understand correctly.
There was a problem hiding this comment.
@jzone3 totally, yes
src/promptlayer/index.ts
Outdated
|
|
||
| const api_key = process.env.PROMPTLAYER_API_KEY!; | ||
|
|
||
| const handler: ProxyHandler<typeof BaseOpenAI> = { |
There was a problem hiding this comment.
Should this be openAIHandler? What happens when we add Anthropic?
There was a problem hiding this comment.
Still need to figure out how we are going to do lazy loading that works across the board, if we end up with this syntax, then we will most likely create a function with generic implementation which we should be able to use with anthropic as well.
src/mocks/handlers.ts
Outdated
| import { rest } from "msw"; | ||
|
|
||
| export const handlers = [ | ||
| rest.post("https://api.promptlayer.com/track-request", (req, res, ctx) => { | ||
| return res( | ||
| ctx.status(200), | ||
| ctx.json({ | ||
| request_id: 1234567890, | ||
| }) | ||
| ); | ||
| }), | ||
| ]; |
There was a problem hiding this comment.
What is this? Why are we committing it?
src/mocks/server.ts
Outdated
| import { handlers } from "@/mocks/handlers"; | ||
| import { setupServer } from "msw/node"; | ||
| import { afterAll, afterEach, beforeAll } from "vitest"; | ||
|
|
||
| export const server = setupServer(...handlers); | ||
|
|
||
| // Start server before all tests | ||
| beforeAll(() => server.listen({ onUnhandledRequest: "error" })); | ||
|
|
||
| // Close server after all tests | ||
| afterAll(() => server.close()); | ||
|
|
||
| // Reset handlers after each test `important for test isolation` | ||
| afterEach(() => server.resetHandlers()); |
There was a problem hiding this comment.
What is this? We shouldn't have tests committed
src/promptlayer/index.ts
Outdated
| return new Proxy(newTarget, handler); | ||
| }, | ||
| get: (target, prop, receiver) => { | ||
| const value = target[prop as keyof typeof target]; |
There was a problem hiding this comment.
target[prop as keyof typeof target]
This can't be normal
tests/index.test.ts
Outdated
| @@ -1,3 +1,8 @@ | |||
| import { describe, expect, it } from "vitest"; | |||
There was a problem hiding this comment.
This repo is going to be public. We can put tests in another repo
fix #4
This PR adds proxy support for
openainode package. Also adds some unit tests.