Skip to content

Data Explorer: Preserve non-file URIs when creating DuckDB clients #7533

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

Merged
merged 1 commit into from
May 6, 2025

Conversation

atheriel
Copy link
Contributor

@atheriel atheriel commented May 5, 2025

Previously we encoded only the original path in the data explorer, losing the scheme (and other URI attributes) and fundamentally assuming that we could only open local files.

This commit instead encodes the original URI in its entirety and adds a helper to retrieve it, which makes it possible to use the Open as Plain Text button even with files backed by a virtual filesystem provider.

Double-encoding the URI here is pretty gross, of course, but it's consistent with what we were already doing.

This is the last piece of #7351.

e2e: @:data-explorer

Release Notes

New Features

  • N/A

Bug Fixes

  • N/A

QA Notes

We should ideally confirm that Open as Plain Text works when the data explorer is opened with a file backed by a virtual filesystem provider.

@atheriel atheriel requested a review from wesm May 5, 2025 17:24
Copy link

github-actions bot commented May 5, 2025

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:data-explorer

readme  valid tags

Previously we encoded only the original path in the data explorer,
losing the scheme (and other URI attributes) and fundamentally assuming
that we could only open local files.

This commit instead encodes the original URI in its entirety and adds a
helper to retrieve it, which makes it possible to use the `Open as Plain
Text` button even with files backed by a virtual filesystem provider.

Double-encoding the URI here is pretty gross, of course, but it's
consistent with what we were already doing.

This is the last piece of #7351.

Signed-off-by: Aaron Jacobs <[email protected]>
@atheriel atheriel force-pushed the feature/aj-open-as-plain-text-preserve-uris branch from cfe26f4 to b653280 Compare May 6, 2025 13:24
@atheriel
Copy link
Contributor Author

atheriel commented May 6, 2025

e2e issues seem to be resolved.

Copy link
Contributor

@wesm wesm left a comment

Choose a reason for hiding this comment

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

+1

@wesm wesm merged commit 5e4799d into main May 6, 2025
8 checks passed
@wesm wesm deleted the feature/aj-open-as-plain-text-preserve-uris branch May 6, 2025 18:17
@github-actions github-actions bot locked and limited conversation to collaborators May 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants