opt-in metadata

[judepayne]

@borkdude, A sketch of how opt-in metadata on args (for just transit+json) might look. (no testing, or README changes yet).

The idea is that in a pod's vars you can set "metadata?" "true" e.g.: for testing, I had a very simple instaparse pod function:

(defn round-trip [x] x)

and set metadata?:

{"name" "round-trip" "metadata?" "true"}

Then I can pass in metadata e.g. from test.clj:

user> (insta/round-trip (with-meta [2] {:my-meta 2}))
[2]
user> (meta (insta/round-trip (with-meta [2] {:my-meta 2})))
{:my-meta 2}

Performance testing looks fine:

10000 times calling `round-trip` - warmed up

current release of pod lib  ~470ms
PR version:
round-trip fn with meta-on  ~540 ms
round-trip fn with meta-off ~470 ms  --> no penalty

[judepayne]

Looks like there's something wrong with the circleci

[judepayne]

@borkdude Have added documentation to README, put in the 'unreleased' section of the Changelog and added a couple of tests and run all tests.

My argument for including this: There are 3 reasons (as far as I can see) why pods wrapping existing Clojure libraries cannot expose functions directly but must wrap them into client side "code"/code: macros, functions accepting functions and functions expecting metadata. This change does only hit a small number of use cases, but it does help to lower friction by eliminating one of the three :)

BTW I'm guessing circleci must be currently disabled on this. It's set up on my fork and the tests are successful.

[borkdude]

I'm not sure what's wrong with CircleCI but I pushed your branch here:

https://github.com/babashka/pods/tree/judepayne-metadata-on-args

and it's running fine over there. Sometimes Circle behaves weird if the forked repo also added their fork to Circle or sometimes Circle refuses to run for certain users based on their IP (e.g. certain countries / political stuff).

[borkdude]

I'm going to ponder your implementation for a bit. Have you thought about any possible alternatives?

Making a decision matrix sometimes helps seeing trade-offs between alternatives, something that I'm starting to do more. Here is an example:

https://docs.google.com/spreadsheets/d/14bDj85LJwUrImLbt87oz9-_OMfdorU16sT4FqFdBL1s/edit?usp=sharing

One very minor nitpicky thing: I try to avoid question marks for boolean fields, so I prefer :meta true instead of :meta? true.

[judepayne]

It's good to ponder. I am also not sure on this one - whether it's worth it (the use cases really are small in number), or is there a risk which I can't currently see. Thanks for the decision matrix idea. I will also ponder.

In terms of alternatives, I don't have another one in the pods library itself yet. With no change to the pods library I could take the nested data structure holding the nested metadata and decorate the data structure with its own metadata explicitly e.g. {:meta {:top-level 1} :value [:S {:meta {:2nd-level 2} :value ["a" "a" "a" .. and hide that away from the user in the client side wrapper, but then the exposed pod function receiving that would in effect not be useable by anyone, so not completely hidden then and not ideal. A key question then becomes 'can we expect everyone to use the client side wrapper and not the pod directly'.

[borkdude]

Can I invite you to create such a matrix? You can DM your google docs e-mail address to me on Slack or so

[judepayne]

I will do that! Want to ponder alternatives first.

[borkdude]

Decision matrix: https://docs.google.com/spreadsheets/d/1kM0eDfQRPOYyL4ov5R8z2rA8DXu8JJoHrj7XQpcqrp4/edit?usp=sharing

[borkdude]

Some minor things:

• As said previously, I don't use question marks on settings, even if they are booleans, so "foo?" "true" => "foo" "true". This is the same as clojure.core does it: :macro true.
• "read-meta?" "true" => perhaps "arg-meta" "true"? as in serialize metadata on arguments?

(defn bytes->boolean [^"[B" bytes]
  (boolean bytes))

How is this successfully turning a a byte array into a boolean...? Isn't this always returning true since a byte array is always truthy?

.. 93 (defonce vars-with-metadata (atom {}))

Why a global to store vars? Not necessarily a problem, just wondering if we can just store this in the "pod" argument?

I have more feedback tomorrow, but maybe you can already process this.

[judepayne]

fixed the first two. Will await the rest of your feedback

[judepayne]

I choose a global to keep the change better separated; aware pods is a critical piece of the babashka world. Just had a look at storing in the 'pod' argument and is a straightforward change (and means no global). Certainly can change it.

[borkdude]

Thanks, let's to that. I got into a rabbit hole with something else today, but I hope to revisit this tomorrow, it's the next thing on my list.

[judepayne]

there we go

[borkdude]

@judepayne Thanks a lot, excellent work!

[judepayne]

Thanks! how long does it typically take to get released? (my understanding is that pods is built into babashka?)

[borkdude]

That's right, pods is built into bb. Bb is released monthly usually. So I think in two weeks or so.

You can try dev builds like this:

bash <(curl https://raw.githubusercontent.com/babashka/babashka/master/install) --dev-build --dir /tmp

And then:

/tmp/bb ...

[judepayne]

got it. thanks