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

Explore remote datasets in datastore #7800

Merged
merged 12 commits into from
May 22, 2024
Merged

Explore remote datasets in datastore #7800

merged 12 commits into from
May 22, 2024

Conversation

fm3
Copy link
Member

@fm3 fm3 commented May 14, 2024

The exploring of remote and local datasets now happens entirely in the datastore module. The wk side only forwards the request (after storing the credentials in postgres and then sending only the credential IDs).

This means that the user can select which datastore should handle the exploring.

It also means that for local datasets, the directory whitelist config setting does not have to work on the wk server side, only on the datastore side. Thus, this feature is now also available for multi-datastore setups where the wk side does not have access to the same file systems.

TODO

  • Frontend: Allow selecting datastore, pass its name to /datasets/exploreRemote parameters (needs to be equal for all items in the list)

Steps to test:

  • Explore remote dataset, should work
  • Explore local dataset with file://, should yield error message suggesting you add the directory to the config setting
  • after doing that it should work
  • If you want to be really diligent: test this with a datastore on a different file system (but I’d say it’s ok to see if this works in production)

Issues:


  • Updated changelog
  • Removed dev-only changes like prints and application.conf edits
  • Considered common edge cases
  • Needs datastore update after deployment

@fm3 fm3 self-assigned this May 14, 2024
@fm3 fm3 changed the title WIP: explore remote datasets in datastore Explore remote datasets in datastore May 16, 2024
@fm3 fm3 marked this pull request as ready for review May 16, 2024 13:39
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.

I took a look at the backend changes as best as I could. I only found minor suggestions in the frontend code. Once they are fixed, this should be good to go :)

The testing worked fine 🎉

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Thanks for applying my feedback & also fixing dataset_upload_view.tsx.

I retested and it worked 🎉

Lets marge

@fm3 fm3 merged commit e443ee5 into master May 22, 2024
2 checks passed
@fm3 fm3 deleted the explore-per-datastore branch May 22, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

localFolderWhitelist for dataset exploring should work for multi-datastore setup
2 participants