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

[BUG][Client] get_dataset is ommitting part of the path in the URL #4129

Closed
carlosmagan opened this issue Aug 5, 2024 · 4 comments · Fixed by #4137
Closed

[BUG][Client] get_dataset is ommitting part of the path in the URL #4129

carlosmagan opened this issue Aug 5, 2024 · 4 comments · Fixed by #4137
Assignees
Labels
bug Something isn't working

Comments

@carlosmagan
Copy link

Describe the bug
There is a behavioural difference on how to access the URLs in client.get_dataset, client._get_dataset_id_by_name and client.get_dataset_versions, specifically the step where the url is generated.

In get_dataset we have
urljoin(self._base_url, f"/v1/datasets/{quote(id)}/examples")

In _get_dataset_id_by_name we have
urljoin(self._base_url, "/v1/datasets")

In get_dataset_versions we have
url = urljoin(self._base_url, f"v1/datasets/{dataset_id}/versions")

The correct one is the one in get_dataset_versions.

The '/' before v1 implies that the relative url is absolute and must follow the domain at a root level. This is a problem when the base url is something on the line of https://base_domain.com/phoenix because the /phoenix is removed resulting in a final url like https://base_domain.com/v1/datasets/example_id/examples instead of the correct one https://base_domain.com/phoenix/v1/datasets/example_id/examples

To Reproduce
Steps to reproduce the behavior:

  1. Deploy phoenix on an endpoint whose url has some kind of path after the domain
  2. Upload a dataset
  3. Instantiate a phoenix client in a notebook
  4. query the dataset with the get_dataset method

Expected behavior
The path for get_dataset should include the complete base_path
Screenshots
image

@carlosmagan carlosmagan added bug Something isn't working triage issues that need triage labels Aug 5, 2024
@axiomofjoy
Copy link
Contributor

@carlosmagan Thanks for reporting and good find, we'll take a look.

@axiomofjoy axiomofjoy self-assigned this Aug 5, 2024
@axiomofjoy axiomofjoy removed the triage issues that need triage label Aug 6, 2024
@axiomofjoy
Copy link
Contributor

Great catch @carlosmagan, thanks for the detailed explanation.

@axiomofjoy
Copy link
Contributor

This is resolved in arize-phoenix 4.20.0

@carlosmagan
Copy link
Author

Thanks a lot for the quick fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants