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

Support :multipart #26

Open
borkdude opened this issue Sep 19, 2020 · 7 comments
Open

Support :multipart #26

borkdude opened this issue Sep 19, 2020 · 7 comments

Comments

@borkdude
Copy link
Collaborator

No description provided.

johnchristopherjones added a commit to johnchristopherjones/babashka.curl that referenced this issue Sep 19, 2020
@borkdude
Copy link
Collaborator Author

@johnchristopherjones starting to look complete I think? :)

@johnchristopherjones
Copy link

johnchristopherjones commented Sep 22, 2020

@borkdude It's kinda close but I have some additional tests locally that are failing. I also realized that explicitly following clj-http semantics for :multipart leads to some problems. Is it more important to have fidelity to curl (I think so) or drop-in compatibility with clj-http?

  1. So, curl can do more with :multipart than clj-http apparently supports. This mismatch means that drop-in compatibility with clj-http comes at the cost of fidelity with curl. In particular, curl allows you to attach any number of named metadata fields (called "headers") to a keypair, while clj-http only supports mime-type metadata. This is particularly vexing because you can pass a seq of maps via :multipart and clj-http assigns special meaning to :name, :part-name, :content, and :mime-type keys, swallows any :encoding key for its own use, and doesn't support any metadata except :mime-type which it remaps to type metadata.

    • Because I'm effectively ignoring namespaces and literally matching keys, I could support drop-in compatibility with clj-http with un-namespaced keys and colliding names that are namespaced would pass through as headers. That is {:name "foo" :content "bar" ::name "baz"} produces ["--form", "foo=bar;name=baz"]. However, I think this is really obtuse and it's way more obvious to just break with drop-in compatibility.
  2. The second half of the curl manpage about multipart is about extensions to the syntax to support multipart SMTP. I haven't explicitly dealt with this yet. I think the smart thing is to extend the :multipart to take a seq and vary depending on the seq's structure:

    1. If it's a string, pass it literally to curl's -F, allowing you to do =( and =) like curl requires. (extension over clj-http)
    2. If it's a 2-tuple like a MapEntry those are the key and value. (clj-http compatible) You can also, for example provide [nil, "("] to begin an SMTP multipart (=().
    3. If it's a 3-tuple, the third item is a map of headers (extension over clj-http) aside: flirting with the idea of making this hiccup-compatible, but that's probably obscene.
    4. If it's a map, it works exactly like clj-http and you can have your key collisions. (clj-http compatible) You can also namespace your keys to make them definitely-headers (extension over clj-http).
  3. Finally, clj-http takes a bunch of care with encoding that I'm not sure I need to follow, because clj-http is really just trying to speak to Apache and babashka.curl is really just constructing a command line argument and encoding concerns have already been settled by the time we get here. This is just my hypothesis though; I haven't grappled with it.

@borkdude
Copy link
Collaborator Author

@johnchristopherjones babashka.curl only says it's inspired by clj-http but there's no need to be 100% compatible. The main goal is to write Clojure to work with curl, in a way that's idiomatic and hopefully familiar to Clojure users.

If curl supports more in this area than clj-http (admittedly, I haven't looked into this as deep as you have) I think your proposal makes sense. Feel free to go ahead with your proposed solution!

@johnchristopherjones
Copy link

@borkdude I don't think I'm going to have enough time to finish this in time for the release today. Sorry about that. I don't mind if someone else wants to push this along.

@borkdude
Copy link
Collaborator Author

@johnchristopherjones This doesn't have to make it into this week's release per se. The library can also be run from source with babashka, if people want to use the new feature and else they just have to wait a month.

@borkdude
Copy link
Collaborator Author

@johnchristopherjones I didn't think it would be this involved but I'd rather wait and have it the proper way like you suggested. Thanks for the work so far.

@borkdude borkdude changed the title Support :multipart (modelled after clj-http) Support :multipart Sep 24, 2020
@borkdude
Copy link
Collaborator Author

Just released v0.2.1. Next release will be in around 4 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants