search

taoensso / faraday

Amazon DynamoDB client for Clojure
https://www.taoensso.com/faraday
Eclipse Public License 1.0
238 stars 84 forks source link

Fix non-expand mode batch write #109

Closed joelittlejohn closed 4 years ago

joelittlejohn commented 7 years ago

Treat incoming batch of updates as a seq rather than a single value.

Hi @ptaoussanis, apologies for not submitting 010c151 as a PR, I had forgotten that I had full access to this repo :)

I'm trying to disable the 'expand' behaviour (related to #63). I've found that attr-multi-vs appeared to be doing the wrong thing in the 'non-expand' branch - it wasn't treating the incoming batch of updates as a seq and was attempting to convert it directly to a dynamo value. This is working well for me now that I have made this change.

joelittlejohn commented 7 years ago

The logical change in this PR is a little easier to see when w=1:

https://github.com/ptaoussanis/faraday/pull/109/commits/4c3fe3bbc5612dedb88369da5e2e2b9756ee9b93?w=1

belucid commented 5 years ago

@joelittlejohn I'll be looking at this now.

joelittlejohn commented 5 years ago

@belucid great. We've been using this patch in our own branch of Faraday so I think it's pretty solid. It's been a while, but please ask questions if you have any :)

belucid commented 5 years ago

That's great to hear @joelittlejohn... nothing gives me more confidence than successful production use.

I'll ping with any questions.

joelittlejohn commented 5 years ago

Nope, kill it. I was trying to minimize my diff.

On Tue, 4 Dec 2018, 16:16 Sean Johnson <notifications@github.com wrote:

@belucid commented on this pull request.

In src/taoensso/faraday.clj https://github.com/ptaoussanis/faraday/pull/109#discussion_r238731634:

@@ -1079,10 +1079,10 @@ (let [expand? attr-multi-vs? expand? (if (nil? expand?) attr-multi-vs?-default expand?)]

  • (if-not expand?
  • (clj-item->db-item attr-multi-vs-map)
  • (let [;; ensure-coll (fn [x] (if (coll?* x) x [x]))
  • ensure-sequential (fn [x] (if (sequential? x) x [x]))]
  • (let [;; ensure-coll (fn [x] (if (coll?* x) x [x]))

Any reason to keep this line around @joelittlejohn https://github.com/joelittlejohn ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ptaoussanis/faraday/pull/109#pullrequestreview-181345395, or mute the thread https://github.com/notifications/unsubscribe-auth/AAp5MEZSWSJz6-xA41rZyY5vHoHyOlUtks5u1p_fgaJpZM4LyftF .

joelittlejohn commented 5 years ago

Comment killed :+1: