zprint-filter-1.2.0-alpha1

[kkinnear]

The 1.2.0 release is coming along, and is almost complete except for one more (possible) additional feature and whatever bug fixes come up in the coming couple of weeks. These two Graal-vm binaries have passed all of the tests on which they have been tried, but there is some additional testing usually done prior to a release that hasn't been performed on these two binaries. Only the graal-vm binaries and uberjar are included in this alpha -- the library is not included and has not been uploaded to clojars. In addition, the corresponding sources have not yet been pushed to GitHub. The cljdoc documentation has not yet been updated as well.

---

This discussion was created from the release zprint-filter-1.2.0-alpha1.

[arichiardi]

Dear Kim, as per the other conversation, I am attaching my config here and I will append all the pieces of code that seem to have diff with 1.1.2 below and over time.

.zprint.txt

Thanks for all your work and I hope it won't be too much 😄

[arichiardi]

This line is exactly 100 chars (which is my width limit)

                  (sut/register-socket right-skt-with-wrong-topic-with-args [wrong-topic-with-args])

It now gets split, it was not before.

-                  (sut/register-socket right-skt-with-wrong-topic-with-args [wrong-topic-with-args])
+                  (sut/register-socket right-skt-with-wrong-topic-with-args
+                                       [wrong-topic-with-args])

I have many instances of the same behavior.

EDIT: maybe now it's correct and the bug was present before?

[arichiardi]

✔️

[arichiardi]

The following form is reformatted:

(t/deftest require-jwt-token-authentication
  (t/testing "returns 401 Unauthorized with invalid JWT claims"
    (let [opts {:auth {:iss "adf" :secret "qwer"}}
          interceptor (sut/require-jwt-token-authentication opts)]
      (t/is (= 401
               (->> interceptor
                    (tu/execute-interceptors {:request {:headers {"authorization" "Bearer X"}}})
                    :response
                    :status))))))

to

(t/deftest require-jwt-token-authentication
  (t/testing "returns 401 Unauthorized with invalid JWT claims"
    (let [opts {:auth {:iss "adf" :secret "qwer"}}
          interceptor (sut/require-jwt-token-authentication opts)]
      (t/is
       (= 401
          (->> interceptor
               (tu/execute-interceptors {:request {:headers {"authorization" "Bearer X"}}})
               :response
               :status))))))

[kkinnear]

I'm working on this one. I can see why it is happening, but I'm not sure what is going on. Thank you very much for going to the trouble to give me this information. It was great that you found this, as it clearly is a problem! I don't know how many of the other things share this particular problem, but we'll see. Thanks again for the attention to detail and the effort to get me the information!

[arichiardi]

✔️

[arichiardi]

The following form is reformatted (see zprint.edn customization):

(defn on-integration-added
  []
  (interceptors/interceptor
   {:name ::on-integration-added
    :enter
    (fn [ctx]
      {:pre [(contains? ctx ::service-integration)]}
      (let [socket-id (-> ctx
                          :transport/socket
                          :id)
            service-integration (::service-integration ctx)
            added-event (service-integration-added-event service-integration)]
        (-> ctx
            (results/declare-result (results/make-ok "Integration added."
                                                     {:service-integration service-integration}))
            (broadcasts/add-broadcast (broadcasts/broadcast-others added-event socket-id)))))}))

to

(defn on-integration-added
  []
  (interceptors/interceptor
   {:name ::on-integration-added
    :enter (fn [ctx]
             {:pre [(contains? ctx ::service-integration)]}
             (let [socket-id (-> ctx
                                 :transport/socket
                                 :id)
                   service-integration (::service-integration ctx)
                   added-event (service-integration-added-event service-integration)]
               (-> ctx
                   (results/declare-result (results/make-ok "Integration added."
                                                            {:service-integration
                                                             service-integration}))
                   (broadcasts/add-broadcast (broadcasts/broadcast-others added-event
                                                                          socket-id)))))}))

[arichiardi]

✔️

[arichiardi]

The following reformats and I would really love to keep the map on two lines here (haven't customized it yet but was about to):

(defn- find-all
  [{:keys [crux]}]
  (let [crux-node (:node crux)
        all (-> (crux/db crux-node)
                (crux/q '{:find [(pull md [*])]
                          :where [[md :xxxxxxx/type :measurement-definition]]}))
        xf (comp (map first) (map mapper.measurement-definition/crux-entity->model))]
    (into [] xf all)))

to

(defn- find-all
  [{:keys [crux]}]
  (let [crux-node (:node crux)
        all (-> (crux/db crux-node)
                (crux/q
                 '{:find [(pull md [*])] :where [[md :xxxxxxx/type :measurement-definition]]}))
        xf (comp (map first) (map mapper.measurement-definition/crux-entity->model))]
    (into [] xf all)))

[arichiardi]

✔️

[arichiardi]

While I could live with the above ones, this is a bit odd:

(defn- find-by-id
  [{:keys [crux]} id]
  (let [crux-node (:node crux)
        md (crux/q (crux/db crux-node)
                   '{:find [(pull md [*])]
                     :in [md-id]
                     :where [[md :cohesic/type :measurement-definition] [md :crux.db/id md-id]]
                     :limit 1}
                   id)]
    (-> md
        ffirst
        mapper.measurement-definition/crux-entity->model)))

to

(defn- find-by-id
  [{:keys [crux]} id]
  (let
    [crux-node (:node crux)
     md
     (crux/q
      (crux/db crux-node)
      '{:find [(pull md [*])]
        :in [md-id]
        :where [[md :cohesic/type :measurement-definition] [md :crux.db/id md-id]]
        :limit 1}
      id)]
    (-> md
        ffirst
        mapper.measurement-definition/crux-entity->model)))

The former imho reads a lot better.

I wonder if it keeps width into consideration here, cause there are plenty of chars to 100 and it seems to squeeze things unnecessarily.

I have many instances of this, all around crux queries.

[arichiardi]

✔️

[arichiardi]

The following some-> is ok visually but also gets squeezed?

(defn- find-reference-range-and-set*
  "Reverse join a reference range in order to navigate to its ancestors.

  Return a reference range Crux entity."
  [repository reference-range-id]
  (some-> (get-in repository [:crux :node])
          (crux/db)
          (crux/q
           '{:find
             [(pull ?e [* {(:reference-range-set/_reference-ranges {:cardinality :one}) [*]}])]
             :in [?id]
             :where [[?e :cohesic/type :reference-range] [?e :crux.db/id ?id]]}
           reference-range-id)
          not-empty
          ffirst))

to

(defn- find-reference-range-and-set*
  "Reverse join a reference range in order to navigate to its ancestors.

  Return a reference range Crux entity."
  [repository reference-range-id]
  (some->
   (get-in repository [:crux :node])
   (crux/db)
   (crux/q
    '{:find [(pull ?e [* {(:reference-range-set/_reference-ranges {:cardinality :one}) [*]}])]
      :in [?id]
      :where [[?e :cohesic/type :reference-range] [?e :crux.db/id ?id]]}
    reference-range-id)
   not-empty
   ffirst))

[arichiardi]

✔️

[arichiardi]

The following seems to be a better behavior but reporting it here for completeness:

(extend-protocol interface.event-bus/Publisher
 nil
   (publish [_ event]
     (logging/warn "Publisher received nil payload, not publishing."
                   (some->> event
                            :id
                            (hash-map :event-id)))))

to

(extend-protocol interface.event-bus/Publisher
 nil
 (publish [_ event]
   (logging/warn "Publisher received nil payload, not publishing."
       q          (some->> event
                          :id
                          (hash-map :event-id)))))

[arichiardi]

I might tweak my config but this one looks way better this way. Will try to tweak that - I don't have an entry for extend-protocol at the moment.

[arichiardi]

The following seems to reformat because of the :keys in the parameter list (see my .zprint.edn, customization intended for clojure specs only):

(defn selected-protocol-for-indications
  [{:keys [spec]} procedure-id indications]
  (->> {:procedure-id procedure-id :indications indications}
       (sql/op spec queries :selected-protocol-for-indications)
       (map :protocol-id)))

to

(defn selected-protocol-for-indications
  [{:keys [spec]} procedure-id indications]
  (->> {:procedure-id procedure-id
        :indications indications}
       (sql/op spec queries :selected-protocol-for-indications)
       (map :protocol-id)))

[kkinnear]

I was in a hurry yesterday, the beginning of this reply:

This particular piece of code formatted correctly the first time, but a bug I fixed in 1.2.0 caused the change. I have now fixed that.

was supposed to be in a reply to the extend-protocol issue, not the one it ended up on. Sorry about that, I was really pushed for time. The reason it formats the way it does is because extend protocol often has multiple type/class symbols, for example:

(czprint i153m {:parse-string? true})
(extend-protocol XmlNode
 Integer
   (as-xml [this] (str this))
 String
   (as-xml [this] (identity this))
 java.util.Date
   (as-xml [this]
     (-> (java.text.SimpleDateFormat. "yyyy-MM-dd HH:mm:ss")
         (.format this))))

so the indenting tends to look nicer. You can have this if you want:

(czprint i153m {:parse-string? true :extend {:indent 0}})
(extend-protocol XmlNode
 Integer
 (as-xml [this] (str this))
 String
 (as-xml [this] (identity this))
 java.util.Date
 (as-xml [this]
   (-> (java.text.SimpleDateFormat. "yyyy-MM-dd HH:mm:ss")
       (.format this))))

and using your code:

(czprint i153k {:parse-string? true :extend {:indent 0}})
(extend-protocol interface.event-bus/Publisher
 nil
 (publish [_ event]
   (logging/warn "Publisher received nil payload, not publishing."
                 (some->> event
                          :id
                          (hash-map :event-id)))))

Thanks for checking things out, I'll look into the :keys issue now.

[kkinnear]

This seemingly innocuous difference:

  (defn selected-protocol-for-indications
   [{:keys [spec]} procedure-id indications]
-  (->> {:procedure-id procedure-id :indications indications}
+  (->> {:procedure-id procedure-id
+        :indications indications}
        (sql/op spec queries :selected-protocol-for-indications)
        (map :protocol-id)))

caused me to look more deeply into how the entire :next-inner capability was operating. Ultimately, I finally understood where the code I had written for :next-inner :restore would work only in isolation, and didn't integrate correctly when other complex configuration was involved. In order to fix this, I had to completely redesign the restore capability for :next-inner, removing the entire :next-inner :restore capability and replacing it with something a bit more complex to use, but which should work in complex configurations. It was a lot of work, but the code is now much more solid and various pieces of complex configuration play well together. Or at least that is what I've found from my testing. I have uploaded a zprintl-1.2.0-alpha3 which you might try. I expect that you will see that the difference listed immediately above has also disappeared, and that the previous differences will also not be present. At least that is what my testing has shown. This entire experience has shown me how the real world and my testing are only loosely related.

It would be very interested in whatever you find if you try the -alpha3 binary!

Thanks very much all of your work finding the differences and telling me about them! It has made a huge difference to this release, and prevented me from releasing something that I would have had to back out completely, which is never a pleasing thing.

[arichiardi]

Dear @kkinnear, I think we are good now!

I have run the formatter -alpha3 against the whole code base and:

$ make fmt-fix
Processed 547 files, none of which required formatting.

No .zprint.edn got harmed in the process :) Thanks for your explanation. I guess :next-inner is a very complex but powerful feature and was just acting up 🗡️

In any case thanks again and ✔️ here from our side!

[kkinnear]

Thank you very (very!) much for trying this on your codebase. All of the reported problems from your codebase are now enshrined in tests, but there could be something else that I broke. It is heartening to find that there isn't. I'm off to fix "one more bug" and then 1.2.0 will be finished. The good news is that the bug is very localized, and shouldn't affect anything but some edge justification cases.

Thank you again for your attention to detail and willingness to do extra work to report the differences and problems to me. It always makes a positive difference, and this time kept me from releasing something that would have had to be deprecated over a period of time to fix!

[arichiardi]

You put so much effort in zprint this is the least I can do 😄

[lread]

I am happy to add 1.2.0-alpha1 to my rewrite-clj v1 test libs suite when you feel that might be a good idea.

Note that git tags for 1.2.0-alpha1 and 1.1.2 seem to be identical(?).

[kkinnear]

I haven't actually released 1.2.0 yet, and as I don't usually (ever) do an alpha release, I haven't followed customary protocols in pushing all of the code and everything with this alpha. I'll be doing 1.2.0 soon, hopefully by the end of September or so. That said, this alpha really brought out some interesting bugs in 1.2.0, so I may start doing alphas more often!

Thanks for offering though, but we're not quite there!

[arichiardi]

FWIW we bumped to 1.2.0 in our production pipelines and it's was a painless process because of all your work Kim. Thank you again from our team!

[kkinnear]

Thanks for working with me extra on this release. It would have been a real mess if you hadn't tried out the alpha and reported all of the issues that you found. The extra effort on your part made what I hope will be a successful release possible!