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

feat: add default limit to /v1/spans and corresponding client methods #3026

Merged
merged 6 commits into from
Apr 29, 2024

Conversation

axiomofjoy
Copy link
Contributor

@axiomofjoy axiomofjoy commented Apr 28, 2024

  • adds default limit of 1000 to /v1/spans
  • similarly updates default in phoenix.Client and phoenix.Session methods
  • fixes openapi schema, which was in a broken state

resolves #3017

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@axiomofjoy axiomofjoy changed the base branch from main to sql April 28, 2024 22:59
@axiomofjoy axiomofjoy marked this pull request as ready for review April 29, 2024 00:39
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 29, 2024
src/phoenix/server/api/routers/v1/spans.py Outdated Show resolved Hide resolved
src/phoenix/server/api/routers/v1/spans.py Show resolved Hide resolved
Comment on lines +53 to -54
default: default
description: The project name to add the evaluation to
default: default
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix to OpenAPI schema.

Comment on lines +114 to -115
default: default
description: The project name to get evaluations from
default: default
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix to OpenAPI schema

Comment on lines +26 to -25
default: default
description: The project name to get evaluations from
default: default
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix to OpenAPI schema

Comment on lines -107 to -111
"""
summary: Deprecated route for querying for spans, use the POST method instead
operationId: legacyQuerySpans
deprecated: true
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove this route from the OpenAPI schema since it is being deprecated.

@axiomofjoy axiomofjoy merged commit e5698d7 into sql Apr 29, 2024
12 checks passed
@axiomofjoy axiomofjoy deleted the limit-query-spans branch April 29, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M This PR changes 30-99 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[persistence][performance] put limits on span_dataframe
3 participants