2023-09-16 pathom | Clojure Slack Archive

pathom

Mark Wardle 2023-09-16T08:29:11.254419Z

Hi all. The breaking changes to placeholder queries in 2023.08.22 do indeed break at least one of my placeholder queries. I have code that can resolve the preferred terms based on a locale:

(p.eql/process *registry*
                            {:info.snomed.Concept/id 80146002}
                            [{:>/en-US [{(list :info.snomed.Concept/synonyms {:accept-language "en-US"})
                                         [:info.snomed.Description/term]}]}
                             {:>/en-GB [{(list :info.snomed.Concept/synonyms {:accept-language "en-GB"})
                                         [:info.snomed.Description/term]}]}])

Here, the query should return terms for en-US and en-GB - the placeholders use parameters rather than initial data. I see that there's a new opt-in plug-in to turn on prior behaviour, but presumably that affects all resolvers. I'm not sure I really understand the rationale for this change, and for the need to permit custom behaviour for params. In any case, is it better for me to simply add that plug-in, or should I do something different in my resolver on a case-by-case basis when I need to support parameters?

wilkerlucio 2023-09-18T01:31:23.386919Z

thanks! merged on main

🎉 1

Mark Wardle 2023-09-16T08:34:06.501029Z

I pull out the parameters in my resolver using pco/params so not getting these parameters by default seems surprising behaviour.

(seq (some->> (get (pco/params env) :accept-language) (hermes/match-locale svc)))

wilkerlucio 2023-09-16T14:21:08.776049Z

Hi Mark, the change to placeholders shouldn't affect the query in your example, that was an specific feature that sent data from params in the placeholder itself (in your case I see params in things nested on placeholders, but not on the placeholders themselves), this looks might be some but introduced by the changes

wilkerlucio 2023-09-16T14:25:56.583149Z

I tried something minimal but I couldn't reproduce the missing data, on params or forwarding down:

(ns demos.params-under-placeholders
  (:require [com.wsscode.pathom3.connect.indexes :as pci]
            [com.wsscode.pathom3.connect.operation :as pco]
            [com.wsscode.pathom3.interface.eql :as p.eql]))

(pco/defresolver param-dep-resolver
  [env {:keys [other]}]
  {:x (str other " - foo - " (:foo (pco/params env)))})

(def env
  (-> {}
      (pci/register param-dep-resolver)))

(comment
  (p.eql/process env
    {:other "value"}
    ['(:x {:foo 1})
     {:>/oi ['(:x {:foo 1})]}]))

=> {:x "value - foo - 1", :>/oi {:x "value - foo - 1"}}

wilkerlucio 2023-09-16T14:27:11.950089Z

ah, I think I see what you mean, its about the different params having same results, right?

wilkerlucio 2023-09-16T14:27:24.748519Z

like in:

(p.eql/process env
    {:other "value"}
    ['(:x {:foo 1})
     {:>/oi ['(:x {:foo 2})]}])
=> {:x "value - foo - 1", :>/oi {:x "value - foo - 1"}}

wilkerlucio 2023-09-16T14:56:28.727009Z

I made a fix, can you please try to use a version from git sha f953959cab7f2839e8ad74078d6df076f32e3e4f and let me know if it fixes for you?

wilkerlucio 2023-09-16T14:56:45.791819Z

current open PR: https://github.com/wilkerlucio/pathom3/pull/207/files

Mark Wardle 2023-09-16T15:08:35.689759Z

Amazing. Thanks @wilkerlucio - I can confirm my test suite passes with that git sha, as it did with 2023-01-31, and throws errors with 2023-08-22.

Mark Wardle 2023-09-16T15:09:22.440119Z

Sorry I wasn't able to reply earlier and be clearer on what the issue was...

wilkerlucio 2023-09-16T15:10:22.572379Z

no worries, and thanks for testing, I like to add a little optimization to it before merging, because now its detecting the need to reprocess rhe entity inside at runner time, and it can be done at planner

👋🏻 1

wilkerlucio 2023-09-16T15:10:36.742479Z

I’ll let you know once I make the change

Mark Wardle 2023-09-16T15:11:03.279059Z

That's fantastic. Thanks again. Hope you are keeping well.

🙏 1

wilkerlucio 2023-09-16T15:46:51.307849Z

I'm good, hope you too!

wilkerlucio 2023-09-16T15:47:50.156359Z

finished the changes to move the computation for planning, can you try 4e3b485ae8b192fdb29e47f9f087882c07b88d0e please?

🎉 1

wilkerlucio 2023-09-16T15:47:55.957459Z

(also PR updated)

Mark Wardle 2023-09-16T19:42:23.321629Z

Thanks! 4e3b485ae8b192fdb29e47f9f087882c07b88d0e is working for my test suite.

nivekuil 2023-09-16T23:43:02.057739Z

Receiver class com.wsscode.pathom3.interface.smart_map.SmartMap does not define or inherit an implementation of the resolved method 'abstract boolean hasNext()' of interface java.util.Iterator. ^ trying to use a smart map with https://github.com/ribelo/doxa . bug in potemkin maybe?

nivekuil 2023-09-17T00:03:57.289909Z

(instance? java.util.Iterator sm) => true

nivekuil 2023-09-17T00:04:04.796859Z

(instance? java.util.Iterator {}) => false

nivekuil 2023-09-17T00:05:36.378469Z

so smart map has different behavior than {} here https://github.com/ribelo/extropy/blob/master/src/main/ribelo/extropy.cljc#L68

wilkerlucio 2023-09-18T13:14:37.792299Z

hello, I'm not sure about the implications of changing this here, I would expect potemkin to implement that maybe? whats the kind of use case you trying thats failing there? using smart maps as data parts to Doxa?

nivekuil 2023-09-18T16:30:57.255969Z

I was trying to query pathom with datalog, doxa seemed like it might work since it can query maps, but I don't think it will work so easily. So now I have no use case for it

Clojurians Log v2

pathom