-
-
Notifications
You must be signed in to change notification settings - Fork 2
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:Update LangSmith API for Organization and Workspace Management Enhancements #139
Conversation
WalkthroughThe changes in this pull request involve significant updates to the LangSmith API, particularly in the management of organizations, workspaces, and their members. New endpoints have been introduced for creating, updating, and deleting these entities, along with enhancements to their schemas. Additionally, some tenant-related endpoints have been deprecated, and new functionalities for managing roles, permissions, and shared resources have been added. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/libs/LangSmith/openapi.yaml (1)
6675-6680
: Enhance documentation for theinclude_deleted
parameterThe
include_deleted
parameter is consistently added across list endpoints, but its behavior and implications could be better documented.Consider adding more detailed descriptions:
- name: include_deleted in: query schema: title: Include Deleted + description: When true, includes soft-deleted workspaces in the response. Default is false to maintain backward compatibility. type: boolean default: false
Also applies to: 7597-7603
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (6)
src/libs/LangSmith/Generated/LangSmith.ITenantClient.ListTenants.g.cs
is excluded by!**/generated/**
src/libs/LangSmith/Generated/LangSmith.IWorkspacesClient.ListWorkspaces.g.cs
is excluded by!**/generated/**
src/libs/LangSmith/Generated/LangSmith.Models.AppSchemasTenant.g.cs
is excluded by!**/generated/**
src/libs/LangSmith/Generated/LangSmith.Models.TenantForUser.g.cs
is excluded by!**/generated/**
src/libs/LangSmith/Generated/LangSmith.TenantClient.ListTenants.g.cs
is excluded by!**/generated/**
src/libs/LangSmith/Generated/LangSmith.WorkspacesClient.ListWorkspaces.g.cs
is excluded by!**/generated/**
📒 Files selected for processing (1)
src/libs/LangSmith/openapi.yaml
(7 hunks)
🔇 Additional comments (2)
src/libs/LangSmith/openapi.yaml (2)
7614-7619
: LGTM! Well-structured error response schema
The error response follows OpenAPI best practices with proper validation error handling and security requirements.
6675-6680
: Verify consistent implementation of soft-delete functionality
The addition of soft-delete functionality is a significant change. Let's verify its consistent implementation across the API.
Also applies to: 7597-7603, 19564-19564, 19586-19588
✅ Verification successful
Soft-delete functionality is consistently implemented
Based on the verification results:
-
The
include_deleted
parameter is properly implemented in list endpoints for:- Tenants (
/api/v1/tenants
) - Workspaces (
/api/v1/workspaces
)
- Tenants (
-
The
is_deleted
field is correctly defined in relevant schemas for:- Tenant objects
- Workspace objects
-
All delete operations are properly defined with appropriate security requirements and error handling.
The soft-delete implementation follows a consistent pattern across the API with:
- Query parameter for including deleted items in list operations
- Schema support for tracking deletion status
- Proper endpoint definitions for delete operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent implementation of soft-delete functionality
# Check if all list endpoints have include_deleted parameter
echo "Checking list endpoints for include_deleted parameter..."
rg -A 5 "get:" | rg -B 5 "parameters:" | rg "include_deleted"
# Check if all relevant schemas have is_deleted property
echo "Checking schemas for is_deleted property..."
rg -A 5 "type: object" | rg -B 5 "is_deleted"
# Check for potential missing error handling
echo "Checking error responses for endpoints with include_deleted..."
rg -A 10 "include_deleted" | rg "responses:" -A 10 | rg "422"
Length of output: 516
Script:
#!/bin/bash
# Let's try a different approach to verify soft-delete implementation
# Search for include_deleted parameter definitions
echo "Searching for include_deleted parameter definitions..."
rg -l "include_deleted"
# If found, let's examine each file's content around include_deleted
echo "Examining context around include_deleted..."
rg -B 5 -A 10 "include_deleted"
# Search for delete-related operations
echo "Searching for delete operations..."
rg -B 5 -A 10 "delete:"
# Search for is_deleted in schema definitions
echo "Searching for is_deleted in schemas..."
rg -B 5 -A 5 "is_deleted:"
Length of output: 56936
@@ -19542,6 +19561,7 @@ components: | |||
- created_at | |||
- display_name | |||
- is_personal | |||
- is_deleted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider making is_deleted
optional with a default value
Making is_deleted
a required field could break existing clients. Consider:
- Making it optional by removing it from the required fields list
- Adding a default value of
false
- - is_deleted
type: object
properties:
is_deleted:
title: Is Deleted
type: boolean
+ default: false
Additionally, consider extracting common soft-delete properties into a shared component schema to reduce duplication.
# Add to components/schemas
SoftDeleteProperties:
type: object
properties:
is_deleted:
title: Is Deleted
type: boolean
default: false
# Then in your schemas
allOf:
- $ref: '#/components/schemas/SoftDeleteProperties'
Also applies to: 19586-19588, 20523-20523, 20545-20547
Summary by CodeRabbit
New Features
Bug Fixes