fix: add wrapper methods for dataset runs with proper URL encoding#1496
Closed
s1st wants to merge 2 commits intolangfuse:mainfrom
Closed
fix: add wrapper methods for dataset runs with proper URL encoding#1496s1st wants to merge 2 commits intolangfuse:mainfrom
s1st wants to merge 2 commits intolangfuse:mainfrom
Conversation
Contributor
Additional Comments (1)
The test should use the new wrapper method instead: This ensures consistency with the fix being introduced in this PR and makes the test more robust for datasets with special characters in their names. Prompt To Fix With AIThis is a comment left during a code review.
Path: tests/test_datasets.py
Line: 223:223
Comment:
This test still uses the direct API call `langfuse.api.datasets.get_runs(dataset_name)` which bypasses the URL encoding wrapper. This means if someone creates a dataset with slashes in the name, this test would fail with a 404 error.
The test should use the new wrapper method instead:
```suggestion
runs = langfuse.get_dataset_runs(dataset_name)
```
This ensures consistency with the fix being introduced in this PR and makes the test more robust for datasets with special characters in their names.
How can I resolve this? If you propose a fix, please make it concise. |
Fixes langfuse/langfuse#10184 Added wrapper methods in the Langfuse client that properly URL-encode dataset and run names before passing to the Fern API client: - get_dataset_run(): Fetch a specific dataset run by name - get_dataset_runs(): Fetch all runs for a dataset - delete_dataset_run(): Delete a dataset run This enables support for folder-format dataset names containing slashes (e.g., "folder/subfolder/dataset") which previously returned 404 errors due to double URL encoding. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
9062bf9 to
d71efd5
Compare
6720a09 to
89222dd
Compare
Contributor
|
Thanks a lot for raising this! Closing this PR as a duplicate of #1453 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes langfuse/langfuse#10184
When datasets have folder-format names containing slashes (e.g.,
folder/subfolder/dataset), the SDK was returning 404 errors for get_run, get_runs, and delete_run operations because the Fern-generated API client doesn't properly URL-encode slashes in path parameters.This PR adds wrapper methods in the main Langfuse client that properly URL-encode dataset and run names before passing them to the Fern client, following the same pattern already used for get_dataset().
Changes
Test plan
Important
Adds wrapper methods in
client.pyfor URL encoding dataset and run names, supporting folder-format names, with tests intest_datasets.py.get_dataset_run(),get_dataset_runs(), anddelete_dataset_run()wrapper methods inclient.pyto handle URL encoding for dataset and run names with slashes.folder/subfolder/dataset).test_datasets.pyto verify functionality with folder-format dataset names.This description was created by
for 9062bf9. You can customize this summary. It will automatically update as commits are pushed.
Disclaimer: Experimental PR review
Greptile Overview
Greptile Summary
This PR adds three wrapper methods to handle URL encoding for dataset run operations when dataset/run names contain slashes (folder-format names like
folder/subfolder/dataset).Changes Made
New Wrapper Methods in
client.py:get_dataset_run()- Fetches a specific dataset run with proper URL encodingget_dataset_runs()- Lists all runs for a dataset with proper URL encodingdelete_dataset_run()- Deletes a dataset run with proper URL encodingAll three methods follow the same pattern as the existing
get_dataset()wrapper:_url_encode()(withoutis_url_param=Truesince these are path parameters, not query parameters)New Tests in
test_datasets.py:Implementation Details
The fix addresses the root cause: the Fern-generated API client passes parameters directly to httpx without encoding. For path parameters embedded in URLs (like
/datasets/{name}/runs/{runName}), manual URL encoding is required before passing to the Fern client.The
_url_encode()method usesurllib.parse.quote(url, safe="")to encode all characters including slashes. Thesafe=""parameter is critical - without it, slashes would not be encoded, and URLs like/datasets/folder/subfolder/dataset/runs/run1would be misinterpreted by the server.Issue Found
One existing test still uses the direct API call
langfuse.api.datasets.get_runs()instead of the new wrapper method, which could cause failures for datasets with special characters in their names.Confidence Score: 4/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant User participant Langfuse as Langfuse Client participant Wrapper as Wrapper Method participant URLEncode as _url_encode() participant FernAPI as Fern API Client participant HTTPx as httpx participant Server as Langfuse Server User->>Langfuse: get_dataset_run("folder/dataset", "run/name") Langfuse->>Wrapper: Call wrapper method Wrapper->>URLEncode: _url_encode("folder/dataset") URLEncode-->>Wrapper: "folder%2Fdataset" Wrapper->>URLEncode: _url_encode("run/name") URLEncode-->>Wrapper: "run%2Fname" Wrapper->>FernAPI: api.datasets.get_run(encoded_dataset, encoded_run) FernAPI->>HTTPx: request("api/public/datasets/folder%2Fdataset/runs/run%2Fname") HTTPx->>Server: GET /api/public/datasets/folder%2Fdataset/runs/run%2Fname Server-->>HTTPx: 200 OK with DatasetRunWithItems HTTPx-->>FernAPI: Response FernAPI-->>Wrapper: DatasetRunWithItems Wrapper-->>Langfuse: DatasetRunWithItems Langfuse-->>User: DatasetRunWithItems(2/5) Greptile learns from your feedback when you react with thumbs up/down!