search

Netflix / PigPen

Map-Reduce for Clojure
Apache License 2.0
566 stars 55 forks source link

Remove unused bytes->json #166

Closed kurtharriger closed 9 years ago

kurtharriger commented 9 years ago

When we attempted to upgrade to pigpen 3.0 we were getting the following confusing error at compile time: Exception in thread "main" java.lang.ClassNotFoundException: pigpen.pig.runtime, compiling:(pigpen/pig/script.clj:138:13) ...

The script and runtime were both in same jar file and was quite confusing. We got same error trying to require pigpen.pig.runtime in the repl and determined the issue was due caused by bytes-json-> referencing a method that no longer exists.

read-json was replaced by read-str and deprecated from clojure/data.json in v0.2.1. https://github.com/clojure/data.json/commit/6ee71009946731d89ef8f98e7b659fa82443b6a2

It looks as if you are using v0.2.5. In theory it was moved to another file and loaded into the ns for compatibility, so it should still exist although deprecated. However, when I require clojure.data.json from a repl it does not appear to define read-json despite the call in to load the compat shim and for whatever reason is the source of the above ClassNotFoundException. Updating this to read-str resolved the issue.

This method was not used anywhere in pigpen so I removed the method entirely rather than update it. If there is some reason it should be kept then it should be changed to read-str.

mbossenbroek commented 9 years ago

LGTM - I think we used to use that for our internal extensions, but it's no longer used anywhere. Do you need a release soon? I was planning to do some work & release in a couple days - does that work for you?

theJohnnyBrown commented 9 years ago

Thanks! that's great. We can certainly use the snapshot for a few days.

kurtharriger commented 9 years ago

I was intending to update the pom identifier to com.rallydev/pigpen "3.1" and deploy it to our internal repository. I'm bit uncertain where this is specified in gradle however?

cloudbees-pull-request-builder commented 9 years ago

NetflixOSS » PigPen » PigPen-pull-requests #12 SUCCESS This pull request looks good

mbossenbroek commented 9 years ago

IIRC, they changed it to derive that automatically these days. I'm not sure how to override it, but I can check on that if need be. I do know that it published snapshots to jfrog here: https://oss.jfrog.org/oss-snapshot-local/com/netflix/pigpen/pigpen/0.3.1-SNAPSHOT/

Are you able to make use of that build?

kurtharriger commented 9 years ago

Ah yes, the SNAPSHOT build will work for us. Thanks!