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

Move Typescript SDK to use our API v2 #500

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

andresgutgon
Copy link
Contributor

@andresgutgon andresgutgon commented Oct 23, 2024

What?

This change is incremental and everyone in v1 should be able to work without any changes. If they opt-in to use API v2, they need to pass apiVersion: 'v2'. And if they want to run a prompt without streaming they need to pass stream: false

TODO

  • Maybe test somehow v2 and streaming: false I did an ./examples playground in the monorepo to use our sdk
  • Fix gateway tests v2 with errors
  • Fix gateway tests v2 with valid response. I changed the output to match the stream version that the SDK is already returning
  • Implement v2 run method
  • Add new tests for non-streaming
  • Undo refactor of different sdk versions
  • QA playground keeps working with v1
  • Document stream: boolean in docs

IMPORTANT

  • Test production build. I introduced a new package @latitude-data/constants. In tired of not being able to have a unified source of truth for enums and types. This is the first stone

@andresgutgon andresgutgon changed the title Allow SKD users use our API v2 Allow LSD users use our API v2 Oct 23, 2024
@andresgutgon andresgutgon changed the title Allow LSD users use our API v2 Allow SDK users use our API v2 Oct 23, 2024
beforeAll(() => {
sdk = new Latitude(FAKE_LATITUDE_SDK_KEY, {
__internal: {
retryMs: 10,
Copy link
Contributor Author

@andresgutgon andresgutgon Oct 24, 2024

Choose a reason for hiding this comment

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

Faster test. We were spending 3 seconds in the retry test.

@andresgutgon andresgutgon force-pushed the feature/enable-api-v2-call-from-sdk branch from 14e1d1f to 591e040 Compare October 24, 2024 14:33
@@ -0,0 +1,29 @@
import { Latitude } from '@latitude-data/sdk'

async function makeSdkRequest() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A playground to test in the development of the SDK. This makes our job easier

@andresgutgon andresgutgon force-pushed the feature/enable-api-v2-call-from-sdk branch 2 times, most recently from acc73cc to c21921e Compare October 25, 2024 09:59
errorCode: ApiResponseCode
dbErrorRef?: DbErrorRef
}
export type ApiResponseCode = RunErrorCodes | ApiErrorCodes | LatitudeErrorCodes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unified list of codes of all the things that can go wrong in an api call

@@ -52,6 +52,7 @@
"@faker-js/faker": "^8.4.1",
"@latitude-data/compiler": "workspace:^",
"@latitude-data/env": "workspace:*",
"@latitude-data/constants": "workspace:*",
Copy link
Contributor Author

@andresgutgon andresgutgon Oct 25, 2024

Choose a reason for hiding this comment

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

import * as AI from '@latitude-data/constants/ai'

Next step in another moment is to move all constants that are from

  1. Vercel AI
  2. Compiler

Into constants so sharing this types between gateway and sdk and core is perfect and no one depends on the others

@@ -3,11 +3,12 @@
"compilerOptions": {
"moduleResolution": "Bundler",
"outDir": "./dist",
"rootDir": ".",
"rootDir": "../../",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes .d.ts generated file not complain. It can find constants

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not used, please confirm @geclos I think this was created by you

@andresgutgon andresgutgon changed the title Allow SDK users use our API v2 Move Typescript SDK to use our API v2 Oct 28, 2024
@andresgutgon andresgutgon force-pushed the feature/enable-api-v2-call-from-sdk branch 2 times, most recently from 6eacf54 to 2beea93 Compare October 28, 2024 12:02
return c.json(data)
const response = await result.response.then((r) => r.unwrap())
const body = documentRunPresenter(response).unwrap()
return c.json(body)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an important change. The meet is in documentRunPresenter

noExternal: [
'@latitude-data/env',
'@latitude-data/core',
'@latitude-data/constants',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added @latitude-data/constants to all our apps. This package would be the common package without other dependencies where we put all the shared constants and types.

)
})
})
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I covered all the new code here

@andresgutgon andresgutgon force-pushed the feature/enable-api-v2-call-from-sdk branch from 2beea93 to 900bccc Compare October 28, 2024 12:31
@@ -1,6 +1,6 @@
{
"name": "@latitude-data/sdk",
"version": "0.0.18",
"version": "1.0.0-beta.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed with this: pnpm version premajor --preid beta

geclos
geclos previously approved these changes Oct 28, 2024
Copy link
Collaborator

@geclos geclos left a comment

Choose a reason for hiding this comment

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

LGTM

csansoon
csansoon previously approved these changes Oct 28, 2024
@andresgutgon andresgutgon dismissed stale reviews from csansoon and geclos via 053c483 October 28, 2024 16:02
@andresgutgon andresgutgon force-pushed the feature/enable-api-v2-call-from-sdk branch 2 times, most recently from 053c483 to 47f8b17 Compare October 28, 2024 16:04
@andresgutgon andresgutgon force-pushed the feature/enable-api-v2-call-from-sdk branch 2 times, most recently from 60384f1 to aeeaab4 Compare October 28, 2024 16:11
API v2 is the new version and from now on our typescript sdk will use
it. This change is necessary to allow users call the gateway in a
non-streaming way which in most cases is what they need
@andresgutgon andresgutgon force-pushed the feature/enable-api-v2-call-from-sdk branch from aeeaab4 to cfa27cd Compare October 29, 2024 08:22
@andresgutgon andresgutgon merged commit 712fbdf into main Oct 29, 2024
3 checks passed
@andresgutgon andresgutgon deleted the feature/enable-api-v2-call-from-sdk branch October 29, 2024 09:53
@github-actions github-actions bot locked and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants