Skip to content

Add from/load/to/save <uri/file> #3608

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 8 commits into from
Nov 17, 2023
Merged

Add from/load/to/save <uri/file> #3608

merged 8 commits into from
Nov 17, 2023

Conversation

eliaskosunen
Copy link
Contributor

@eliaskosunen eliaskosunen commented Oct 31, 2023

Supersedes #3500, related to https://github.com/tenzir/issues/issues/328

In this PR:

  • OP <uri> is mapped to OP <uri-scheme> <rest-of-uri>, where OP is one of (from, load, to, save), and <uri> is a RFC 3986 -compliant URI.
  • OP <something-else> is mapped to OP file <something-else> if <something-else> isn't a possible plugin name (doesn't conform to the pattern [A-Za-z0-9-_]*)

This effectively means, that the following mappings are implemented:

  • from https://example.com/foo -> from https example.com/foo
  • from gs://foo/bar -> from gcs gs://foo/bar
  • from file:///tmp/file -> from file /tmp/file
  • from /tmp/file -> from file /tmp/file
  • from ./local-file -> from file ./local-file
  • from local-file -> error (no loader local-file found)
  • from https -> error (no URL given to https loader)

The logic implemented in this PR is as follows:
image

This current implementation accepts from https:foo.bar/baz as from https foo.bar/baz. It's strictly not RFC 3986 -compliant, but I don't think it's that big of an issue.

  • Implement from <uri> [options...] [read <parser>]
  • Implement from <path> for from file <path>
  • Same for load
  • Same for to and save
  • Consider some loader restrictions + remapping (e.g., for gcs)

@eliaskosunen eliaskosunen added feature New functionality connector Loader and saver operator Source, transformation, and sink labels Oct 31, 2023
@eliaskosunen eliaskosunen mentioned this pull request Oct 31, 2023
5 tasks
@eliaskosunen eliaskosunen changed the title Add from <uri> and from <file> Add from <uri/file> and load <file> Nov 1, 2023
@eliaskosunen eliaskosunen changed the title Add from <uri/file> and load <file> Add from <uri/file> and load <uri/file> Nov 1, 2023
@eliaskosunen eliaskosunen force-pushed the topic/from-uri-2 branch 3 times, most recently from af3ef1a to 1e676b8 Compare November 1, 2023 14:11
@eliaskosunen eliaskosunen changed the title Add from <uri/file> and load <uri/file> Add from/load/to/save <uri/file> Nov 1, 2023
@eliaskosunen eliaskosunen marked this pull request as ready for review November 1, 2023 15:39
@eliaskosunen eliaskosunen marked this pull request as draft November 3, 2023 11:47
@eliaskosunen eliaskosunen force-pushed the topic/from-uri-2 branch 3 times, most recently from 864e87e to 5912f12 Compare November 6, 2023 12:43
@eliaskosunen eliaskosunen marked this pull request as ready for review November 6, 2023 12:44
@eliaskosunen eliaskosunen requested review from jachris and tobim November 6, 2023 12:44
@dominiklohmann dominiklohmann removed the request for review from mavam November 6, 2023 13:50
Copy link
Contributor

@jachris jachris left a comment

Choose a reason for hiding this comment

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

Sorry that the review took so long. Had too many things to do, but it's very nice to get this feature finally!

@eliaskosunen eliaskosunen force-pushed the topic/from-uri-2 branch 3 times, most recently from 4836a4c to a3cadae Compare November 14, 2023 14:25
Copy link
Contributor

@jachris jachris left a comment

Choose a reason for hiding this comment

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

The improved UX is very nice! Thanks.

@eliaskosunen eliaskosunen force-pushed the topic/from-uri-2 branch 2 times, most recently from a166438 to 8b7e85c Compare November 16, 2023 15:16
@eliaskosunen eliaskosunen force-pushed the topic/from-uri-2 branch 2 times, most recently from 56cc3b8 to 370da62 Compare November 17, 2023 12:55
@jachris
Copy link
Contributor

jachris commented Nov 17, 2023

Unless there are any remaining issues, I think we can merge this 🚀

cc @dominiklohmann @mavam

@dominiklohmann
Copy link
Member

I think we can merge this, yes.

@eliaskosunen eliaskosunen merged commit 16d4646 into main Nov 17, 2023
@eliaskosunen eliaskosunen deleted the topic/from-uri-2 branch November 17, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connector Loader and saver feature New functionality operator Source, transformation, and sink
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants