Fork me on GitHub
#babashka
<
2022-05-13
>
broy12:05:11

hey! I’m having difficulty adding metosin/ring-swagger and metosin/ring-swagger-ui as a dep to my bb script

{...
:deps {metosin/ring-swagger    {:mvn/version "0.26.2"}
       metosin/ring-swagger-ui {:mvn/version "4.5.0"}}
...}
is it possible at all with babashka? thanks!

borkdude12:05:47

Can you be more specific about the problem?

broy12:05:46

I’m trying to create a swagger file describing the http endpoints supported by the babashka project I’m working in. but I cannot bring the necessary swagger deps in, I get the following error, even in a bb repl:

user=> (require '[babashka.deps :as deps])
nil
user=> (deps/add-deps '{metosin/ring-swagger    {:mvn/version "0.26.2"}})
nil
user=> (require '[ring.swagger.swagger2 :as swagger2])
Could not resolve symbol: clojure.lang.Compiler/CHAR_MAP [at schema/utils.clj:50:3]

borkdude12:05:34

Ah right. So all Java classes must be explicitly enabled. I'll take a look

broy12:05:45

thanks so much

borkdude12:05:42

which bb version are you using? I'm hitting a different issue :)

broy12:05:12

$ bb --version
babashka v0.7.3

broy12:05:36

and min bb version is set to

:min-bb-version "0.6.7"
in the project

borkdude12:05:06

0.8.2 is the newest

broy12:05:55

~upgrading at once

broy12:05:24

yeah, the issue is indeed a bit different 🙂

java.lang.IllegalArgumentException: No matching field found: CHAR_MAP for class clojure.lang.Compiler [at schema/utils.clj:55:8]

borkdude12:05:04

That's still not the error that I'm seeing though. I'm seeing:

Type:     clojure.lang.ExceptionInfo
Message:  Could not resolve symbol: deftype
Phase:    analysis

borkdude13:05:38

I'm looking more into the issue. The first issue is getting schema.core to work, which has a deftype. But with some hacking I got it to work:

$ clojure -M:babashka/dev -e "(require '[schema.core :as s]) (def schema {:foo s/Int}) (prn (s/validate schema {:foo 1})) (prn (s/validate schema {:foo :bar}))"
{:foo 1}
----- Error --------------------------------------------------------------------
Type:     clojure.lang.ExceptionInfo
Message:  Value does not match schema:

borkdude13:05:56

So, the short term answer is, it doesn't work right now and I don't know what will come after schema.core works. I expect more issues, since there are more libraries that aren't supported in bb in there. It will definitely be cool to make schema.core work in bb though, so I'll look deeper into that.

broy13:05:05

makes sense. thanks very much for looking into this :thumbsup:

borkdude13:05:57

Lol, with some hacking I got schema.core to work in babashka:

$ clojure -M:babashka/dev -e "(require '[schema.core :as s]) (def schema {:foo s/Int}) (prn (s/validate schema {:foo 1})) (prn (s/validate schema {:foo :bar}))"
{:foo 1}
----- Error --------------------------------------------------------------------
Type:     clojure.lang.ExceptionInfo
Message:  Value does not match schema:

👏 3
ambrosebs01:05:58

Yes please!!

borkdude08:05:25

I'm hitting a limit with the validation of records, since records are implemented very differently in SCI (since SCI does not create new classes). Would schema still be useful in bb, without the record support?

borkdude08:05:25

This is the success ratio now:

{:test 94, :pass 477, :fail 27, :error 19, :type :summary}

borkdude11:05:19

I merged some improvements to master so schema.core at least partially works :)

ambrosebs01:05:33

Yes I think so, thanks!

borkdude12:05:20

{:test 94, :pass 500, :fail 12, :error 11, :type :summary}

mdave16:05:14

I'm trying to port a lib to bb, but I it seems that calling with-meta on a record casts the type to sci.impl.records.SciRecord and it breaks the internals of the lib. Is there any way to overwrite the with-meta behavior so it keeps the original type like in clj?

(defrecord Person [name])
(def person (map->Person {:name "Smith"}))

(type person)
; user.Person
(prn person)
; #user.Person{:name "Smith"}
; so far so good

(def person-wm (with-meta person {:meta true}))
(type person-wm)
; sci.impl.records.SciRecord

; there is also an issue with printing the record
(prn person-wm)
: java.lang.Class cannot be cast to clojure.lang.Named user 

borkdude17:05:22

SCI uses the metadata on the Person map to indicate the type

borkdude17:05:33

so preferably you should use vary-meta here

borkdude17:05:58

we could fix this by patching with-meta to keep the type data perhaps

borkdude17:05:16

but for now you can fix this by using vary-meta instead

borkdude17:05:01

There might be a better way for SCI to store the metadata, in a separate field probably, but that would make printing maps behave like:

{:a 1 :__sci_type user.Person}
or so, while now the printing works correctly

borkdude17:05:25

Feel free to post an issue about this

mdave17:05:09

Thank you so much, it works now! I overlooked the :type metadata from sci and only tried to re-add the :sci.impl/record true . I think :type is one of the most common keywords in metadata. Either __sci_type , or just an ns qualified keyword would be less prone to conflict with other sort of "types".

mdave18:05:38

I was going to open an issue, but in the meantime I found a comment from you that implies that metadata on SciRecord is obsolete anyway. https://github.com/babashka/sci/blob/246c6c0639d2b55082e06f03611830b99d6286ef/src/sci/impl/records.cljc#L114 Is this an ongoing change or you still prefer to have an issue about this?

borkdude18:05:50

@U014XH9MK55 That change wasn't made yet, but if you create an issue (perhaps in SCI) then we have more reason to work on that :)

💯 1
mdave18:05:27

Thank you too! I have another issue with this lib and I spent a few hours to understand what's going on. Right now I'm just shooting in the dark.

mdave18:05:14

it requires to implement the implement the IFn invoke function on the record. Something like this:

(import [clojure.lang IFn])

(defrecord Person [name]
  IFn
  (invoke [this] (str name)))

mdave18:05:57

but it's erroring out with Could not resolve symbol: invoke

mdave18:05:19

I was trying to extend the SciRecord type with IFn, but no luck

borkdude18:05:15

Unfortunately adding a Java interface to a SCI record isn't yet possible. You should be hitting this: https://github.com/babashka/sci/blob/246c6c0639d2b55082e06f03611830b99d6286ef/src/sci/impl/records.cljc#L71 but it was commented out, that was an error on my part

borkdude18:05:10

you can use reader conditionals to avoid adding IFn to the record

borkdude18:05:25

look in lambdaisland/uri for an example of this, hit the same case

mdave18:05:21

yes, that's what I did

borkdude18:05:57

cool! what's the library btw, anything I might know or a proprietary one?

mdave18:05:13

I'm working on spec-tools

mdave18:05:05

most of the tests on the core now pass, I call this is a success 🙂

borkdude18:05:13

wow, sounds good :)

mdave18:05:28

they wrap clojure spec into a record and implement IFn on that. for example

(def my-integer? (st/spec integer?))
(my-integer? 1) => true

mdave18:05:51

this won't work, but other than that, it's a powerful lib for coercion

borkdude18:05:47

This coercion library also works with bb: https://github.com/exoscale/coax

borkdude18:05:05

But spec-tools is a very welcome addition

borkdude18:05:24

Let me know when you're wrapped up, then we can maybe add the tests to bb's CI

borkdude18:05:41

bb CI runs tests for a ton of libraries that we are supporting once they work with bb

mdave18:05:33

that's great, I will keep you updated!

mdave18:05:53

and probably shoot a few questions if I hit a wall again

👍 1
mdave20:05:50

I found a bug in babashka/spec.alpha but can't open issue on that repo, where should I send it?

borkdude20:05:25

@U014XH9MK55 I enabled issues now

👍 1
mdave14:05:11

@U04V15CAJ , I think I need your help again, I can't wrap my head around how how symbol resolution works in bb/sci (clojure.core/resolve 'fn) should resolve #'clojure.core/fn, but I get nil. On the other hand, (clojure.core/resolve 'get) works as expected.

borkdude15:05:43

The reason for this is that fn is implemented as a special form in SCI

borkdude15:05:56

Is this a problem?

mdave16:05:55

to be honest, I'm not sure. it fails some unit tests, but I don't think it has a big impact in real life use cases

borkdude16:05:41

What would that library do with the result of (resolve 'fn)?

borkdude16:05:48

It's a macro after all

borkdude16:05:15

In CLJS:

cljs.user=> (resolve 'fn)
nil

mdave16:05:40

it uses it to store the source code of the spec predicate

mdave16:05:24

(st/spec (fn [x] (> x 10)))
=> #Spec{:form (clojure.core/fn [x] (clojure.core/> x 10)), :type nil, :leaf? true}

mdave16:05:05

with sci #Spec{:form (fn [x] (clojure.core/> x 10)), :type nil, :leaf? true}

mdave16:05:10

this is already an edge case when you construct a spec-tools/spec directly from a predicate and not from predefined spec

borkdude16:05:35

This works correctly though:

user=> `fn
clojure.core/fn

mdave16:05:35

btw, for cljs they use the cljs.analyzer.api/resolve

borkdude16:05:55

right. what do they do with the result in JVM with the var?

mdave16:05:49

so when you create a spec it stores this predicate in the spec record. it is used directly by describe that tells you what are the underlying predicates

mdave16:05:06

it's also different from clojure.spec.alpha, where there is no resolution either (clojure.spec.alpha/describe (clojure.spec.alpha/spec (fn [] true))) => (fn [] true)

mdave16:05:10

also used by explain

mdave16:05:26

but I need to dig deeper to see if it has any impact on the logic or just extra information for the end user

borkdude18:05:02

Not sure what the problem is. Both with bb and clj I get (fn [] true) ?

mdave19:05:32

I'm sorry, it was a bad example. describe trims the namespace part of the var away. describe* keeps the resolved vars, this should give two different results between bb and clj.

(s/describe* (spec-tools.core/spec (fn [x] (> x 10))))

borkdude19:05:01

I removed the spec-tools one and just used spec:

$ bb -cp src/main/clojure -e "(require '[clojure.spec.alpha :as s]) (s/describe* (s/spec (fn [x] (> x 10))))"
(clojure.core/fn [x] (clojure.core/> x 10))
which gives the same output in clj. Do I understand that this is only an issue with bb and spec-tools and not with bb and clojure.spec.alpha?

borkdude19:05:42

Nonetheless, we could fix that. I just need to know what exactly spec-tools is doing with the resolved var

mdave19:05:43

yes! it's not a spec.alpha issue for sure

borkdude19:05:50

like does it inspect metadata or so

mdave19:05:16

as far as I can tell, there is no other logic relying on this resolution mechanizm

mdave19:05:44

but the whole purpose of describe and form is to give the spec predicate back to the user in data form

mdave19:05:09

so I wouldn't say it's futureproof if we keep it like that

mdave19:05:55

I will look into the fix tomorrow

borkdude20:05:46

yes, but what I'm asking is, how does spec-tools use, in a technical sense, not in a conceptual sense, the var.

borkdude20:05:41

it would actually not be so difficult to support fn as macro in bb

mdave20:05:32

in case of fn, it checks, if the resolved value is a var (yes) and then it converts it to the clojure.core/fn symbol https://github.com/metosin/spec-tools/blob/master/src/spec_tools/impl.cljc#L100

mdave20:05:10

now in bb it's nil, so the :else branch will return from the cond with 'fn . (L104)

mdave20:05:52

furthermore the strip-fn-if-needed relies on the qualified namespace version and won't work with just 'fn https://github.com/metosin/spec-tools/blob/master/src/spec_tools/impl.cljc#L113-L119

mdave20:05:39

also unfn works explicitly with 'clojure.core/fn, but only on the sym level https://github.com/metosin/spec-tools/blob/master/src/spec_tools/impl.cljc#L42-L48

mdave21:05:48

since this only happens with fn, a specific rewrite would be the easiest fix

mdave16:05:46

so far so good

mdave17:05:32

@U04V15CAJ, I came across a minor anomaly when I was debugging in bb repl. I'm not even sure if this is considered to be unexpected, so I'm curious your take on this first

mdave17:05:26

if I connect to a bb nrepl server, create a record, define defmethod print-method on the record and send the symbol to the repl, then the record base data will be printed to the repl and not the one defined in the print-method

borkdude17:05:36

Ah, that probably is already fixed on master. Try with:

bash <(curl ) --version 0.8.3-SNAPSHOT --dir .

mdave17:05:13

indeed! thanks 🙂

borkdude20:05:52

Are you going to try to submit this commit back to Metosin? cc @U055NJ5CC

mdave20:05:53

I think so, that would be the cleanest way moving forward

mdave20:05:52

I couldn't run the cljs tests properly, it depends on phantomjs and I'm not familiar with the setup (yet)

mdave20:05:49

just realized that I've also removed some test.chuck tests and forgot to re-add with the reader conditionals. so I definitely need to push at least another commit and then I can send the pr for that. hopefully towards end of next week I'll be able to do that

mdave20:05:24

btw, for some reason the print-method issue is still ongoing with the spec-tools specs update: just realized that it doesn't work with the explicit prn either, it's going to be a spec-tools and not bb