Fork me on GitHub
#clojure-spec
<
2018-10-25
>
jrychter00:10:49

I really do not like this canned answer ("closed specs are discouraged"). It gets repeated a lot, and yet there are valid scenarios where you do want to restrict the data to exactly what is specified in the spec without manually duplicating lists of keys. Think about API endpoints, or anything that receives data from an untrusted source in general.

seancorfield00:10:44

If you ignore the additional parameters, why do you care whether they are passed in?

seancorfield00:10:32

One valid case I can see for closed specs is when you're about to store data into a database (and you'll get an error if you try to store columns/keys that do not exist in the table). At that point tho', you can extract the list of valid keys from the spec itself and use select-keys to get just the keys you care about. Again, you don't necessarily care that more data was present -- you just need to ensure you don't send it to JDBC.

seancorfield00:10:04

(and of course you could use select-keys on the API input data too, if you wanted)

seancorfield00:10:37

The reality is that most APIs out there silently ignore extra form and query parameters.

seancorfield00:10:03

(which is all a long way of saying I think the canned answer is correct)

taylor00:10:03

I agree there are cases were you need to restrict inputs but I think those are usually near the boundaries of apps, and spec doesn’t prevent this. Other than that I don’t think (in Clojure) your code should “break” if passed a map with some data it doesn’t care about, and I think most projects have much more code on that side of the fence than the other

jrychter00:10:33

If your database is a document database (like mine) and stores your data pretty much in model form (like mine does), and your API serves your ClojureScript app which keeps the same data model, then you very much care about additional parameters. You can pass data pretty much intact between client code and the database, but you do need to make sure that you don't store any extra data that just happens to arrive over the network.

jrychter01:10:24

The manual select-keys solution seems obvious and results in error-prone code (been there done that). A more reasonable approach is to get the list of allowed keys from the spec.

seancorfield03:10:41

When I do select-keys I derive the list of keys to be selected directly from the spec.

jrychter03:10:56

…and that is exactly what I'll be doing.

jrychter01:10:38

I'd also submit that "most APIs" describes "most traditional APIs". My APIs speak EDN and receive Clojure data structures which I verify using spec.

csm01:10:43

possibly s/conform should remove keys not in the spec? I’m not sure of the implications of that, though.

taylor01:10:44

there’s no need to change spec, you can easily get this restrictive behavior if you need it

bbrinck01:10:16

You don’t need to duplicate keys w/ spell-spec

csm01:10:08

maybe I should just ask, though: why doesn’t conform remove keys not in a map spec?

jrychter01:10:28

spell-spec solves a different problem and has a different goal. But I am dealing with the problem myself, I just wanted to point out that whenever people repeat what Rich said about specs intended to be open, there should be a footnote saying "except in cases where you do need to restrict your data, such as in APIs".

bbrinck01:10:39

AFAICT, spell-spec (specifically strict-keys https://github.com/bhauman/spell-spec#spell-specalphastrict-keys ) does solve the problem you describe above. Can you talk more about why it doesn’t?

bbrinck01:10:38

@csm AIUI, conform is useful for destructuring, so presumably there isn’t a need to remove keys.

bbrinck01:10:02

(because you would usually know the keys you want to grab as part of destructuring)

csm01:10:47

:thumbsup: I’m just thinking out loud if there are approaches that would satisfy use cases like the above, but without making specs closed

bbrinck01:10:10

remember that conform also changes the shape of data, so may not apply here if you’re trying to write the input to, say, a document DB

bbrinck01:10:57

(I suppose you could conform/unform if it did remove unknown keys, but that gets a little strange 🙂 )

csm01:10:26

yeah, I thought about that; maybe a fn that does “check this spec, then select-keys” would be an OK addition

andy.fingerhut01:10:39

Doesn't the word "discouraged" in "closed specs are discouraged", vs. e.g. "closed specs are prohibited", convey to you that there might be occasional valid use cases for them?

pizzaspin 4
bbrinck01:10:40

anyway, I am curious why strict-keys from spell-spec doesn’t work here

andy.fingerhut01:10:03

And sure, people can repeat things without knowing the reasons for them, nor the subtler points of when they are good ideas, and when they are not. That is a more general habit of some people to be second-handed in what they say, rather than understanding themselves.

andy.fingerhut01:10:16

not restricted to computing at all.

bbrinck01:10:37

Yeah, I think “discouraged” is an accurate term, especially since I’ve seen a few people jump to use closed specs by default. It turned out that open specs were a better fit.

bbrinck01:10:53

But yes, there are exceptions

csm01:10:21

I, for one, wish spec was mature before I started using schema instead. I’m not a fan of closed specs for APIs

jrychter01:10:04

I do agree that open specs are a good idea.

andy.fingerhut01:10:25

@jrychter I am not trying to jump down your throat. I am asking because I would actually like to learn more about your use case where you want to disallow unrecognized keys. You say you may get these extraneous keys over the network at some document database service, and want to detect this as an error?

jrychter01:10:44

@andy.fingerhut my API receives EDN data, in model form. I want to pass this data to my model code (where extra keys might not matter), and often directly to a document database (RethinkDB in this case). I do not want to store extra keys, for obvious reasons.

jrychter01:10:26

I know that if one uses an SQL database, there is code that needs to re-pack the data, but in my case there is often no such code.

andy.fingerhut01:10:08

Bear with me if I ask something foolish for lack of experience in these kinds of applications, but if the data model is extended in the future, a reason commonly given for open specs is that unless you can somehow arrange for both participants to be upgraded simultaneously (e.g. by shutting down the service while everyone upgrades), then you can't upgrade without lots of errors flying around during the partially updated state, can you?

jrychter01:10:13

An invariant in my case is that the spec describes the data model.

andy.fingerhut01:10:52

In all processes on all systems where the code is running, all of the time, even during updates to the data model?

andy.fingerhut01:10:52

If yes, then it sounds like partially updated systems are never an issue in your use case. That certainly simplifies implementing such things, if so.

jrychter01:10:53

At the moment I mostly use atomic "stop-the-world" migrations, but in general the assumption has been that the spec needs to accomodate partially updated systems. But again: I do agree that open specs are a good idea, I just think that people take Rich's "discouragement" a bit too far and see closed specs as "a bad idea". They are not a bad idea.

andy.fingerhut01:10:06

If open specs are a good idea, then there must be some situations where closed specs are a bad idea.

andy.fingerhut01:10:00

It sounds like Rich & team's experience in consulting is that they have seen multiple scenarios where people wanted to close their specs, and later regretted it.

andy.fingerhut01:10:31

e.g. the first occurrence of the word "consulting" in his talk on spec here: https://github.com/matthiasn/talk-transcripts/blob/master/Hickey_Rich/ClojureSpec.md

andy.fingerhut01:10:07

I mean, I know people can read/hear that and think "Rich means that for 100% of all software everywhere", but I doubt that. But he is speaking in context of an industry where I think there is an over-tendency to make brittle inextensible systems.

taylor01:10:23

spec isn’t particularly unusual in encouraging openness/forward-compatibility either, it’s a concern that has to be addressed in things like protobuf, thrift, etc. too

andy.fingerhut01:10:10

I know I'm in the minority working at Cisco in embedded software that is far too much C code for my taste, but these systems are incredibly brittle. Changing an int to a long is often developer-weeks worth of effort, if it is in a message spread over 3 processes.

😬 4
the2bears03:10:11

This current discussion on closed specs sounds a little too much like using or relying on spec to clean your data. To be fair, though, I don't have any use cases for a closed spec myself and might be missing the real picture. Just seems better to clean the data with a different set of functions and tools.

misha03:10:20

how about not using tool-designed-not-to-solve-A to solve A?

misha03:10:33

if it is such a bummer, just implement closed keys with 1-2 macros yourself, or take one of many available in the libs already.

misha03:10:56

implementing select-spec-keys is trivial, especially when you know what specifically you prefer to do if extra stuff present – ignore, warn, blow up, etc.

orestis06:10:27

I implement this select-spec-keys just yesterday. It’s very straightforward to write and I can deal with assumptions about namespaces, blow up or warm etc etc.

jaihindhreddy-duplicate13:10:17

How can I add test.check to my deps.edn?

jaihindhreddy-duplicate13:10:55

Because it doesn't have any dependencies, will this work?

{:deps
  {clojure/test.check {:git/url ""
                       :sha "f01e3d8ad3317e6dfbeee29332a6122b1a6c12bc"}}}

gfredericks13:10:49

You can use it with the mvn/version just like clojure

jaihindhreddy-duplicate13:10:08

{org.clojure/test.check {:mvn/version "0.10.0-alpha3"}} then?

jaihindhreddy-duplicate13:10:00

Ah, so because test.check publishes artifacts (on maven) I don't need to worry about the fact that it doesn't have a deps.edn

gfredericks13:10:41

Right I guess I should add an empty one

gfredericks13:10:56

I'll do that soon

Alex Miller (Clojure team)14:10:41

you can force the dep type without having a deps.edn

Alex Miller (Clojure team)14:10:49

{:deps
  {clojure/test.check {:git/url ""
                       :sha "f01e3d8ad3317e6dfbeee29332a6122b1a6c12bc"
                       :deps/manifest :deps}}}

Alex Miller (Clojure team)14:10:19

that overrides the manifest detection and forces it to be a deps.edn project, which is tolerant to a missing deps.edn

Alex Miller (Clojure team)14:10:37

and just treats it as a project with no deps

seancorfield16:10:09

A useful trick -- is that documented in the CLI/tools.deps Reference? (specifically that it can be used to treat an arbitrary git-based project "as a project with no deps")