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 pagination #96

Merged
merged 2 commits into from
Jul 17, 2023
Merged

Add pagination #96

merged 2 commits into from
Jul 17, 2023

Conversation

hinthornw
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@dqbd dqbd left a comment

Choose a reason for hiding this comment

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

Smaller nitpicks :)

@@ -62,7 +62,7 @@ export class RunTree implements BaseRun {
api_key: getEnvironmentVariable("LANGCHAIN_API_KEY"),
caller_options: {},
start_time: Date.now(),
end_time: Date.now(),
end_time: null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: shouldn't that be undefined instead? Or maybe we should change end_time data type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes!

js/src/client.ts Outdated
@@ -108,6 +108,14 @@ const raiseForStatus = async (response: Response, operation: string) => {
}
};

export async function toArray<T>(iterable: AsyncIterable<T>): Promise<T[]> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: do we want to expose this function to the outside? (ignore if we're just consuming it ourselves)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm probably not to start ya

Copy link
Collaborator

@dqbd dqbd left a comment

Choose a reason for hiding this comment

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

Smaller nitpicks :)

for await (const dataset of langchainClient.listDatasets({})) {
datasets.push(dataset);
}
if (datasets.some((dataset: any) => dataset.name === datasetName)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Using toArray we would be able to get rid of any

@hinthornw hinthornw merged commit 8aa6cb3 into main Jul 17, 2023
@hinthornw hinthornw deleted the wfh/paginate branch July 17, 2023 14:15
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.

2 participants