moclojer / clj-rq

RQ (Redis Queue) is a simple Clojure package for queueing jobs and processing them in the background with workers. It is backed by Redis and it is designed to have a low barrier to entry
MIT License
15 stars 1 forks source link

implement remaining queue wrapper functions #13

Closed J0sueTM closed 2 weeks ago

J0sueTM commented 2 months ago

As of now, we have wrapped the queue functions pop, push and llen. These are great to the extent of what we've been using clj-rq until now. However, in order to better expand the library into users outside of moclojer, we need to make sure to deliver a good wrapper with great coverage.

J0sueTM commented 1 month ago

@avelino @Felipe-gsilva @matheusfrancisco After tiring myself by trying to compile every redis function from Java to Clojure, I thought of the following:

All Jedis' functions are in this file. What if we could just retrieve this file, and given any function, rebuild it in clojure based on the amount of parameters.

For example:

  public long persist(String key) {

becomes, in our clojure file (probably by building everything with a big macro):

(defn persist [client key]
  (.persist client key])

The main pro of this is that we won't need to focus too much on updating minimal changes when Jedis changes versions. All we need to do is bump the new file and :drum: ... done.

What do you guys think?

avelino commented 1 month ago

I like the idea and wrappers should be built this way, but with the possibility to customize the behavior of some specific function when necessary.

@J0sueTM do you have any idea how to implement this?

J0sueTM commented 1 month ago

@avelino I began scratching around with this idea last week. Basically, what we need to do is load the jedis repo as a submodule in our resources folder which should be cloned and read on development or build time.

After loading the file resource UnifiedJedis.java, we strip lines that begin with public _ myFunction. and transpose then afterwards to clojure function, transferring the function as said in my last comment.

We shall filter out non needed functions, or better yet just keep a list of the functions we need to be transpiled. I prefer using an allowlist, since a blocklist might be prejudicial in the future if a new function is added, and we as contributors forget to add it to the blocklist

We also need to keep a list of functions that have directions. For example push, which can be eitheir left or right.

So to pack it up:

  1. Add the jedis repo as a submodule so the contents are only loaded when necessary
  2. Load the UnifiedJedis.java file and strip out the lines that looks like a public function
  3. Filter out blocked functions based on an internal allowed function list made by ourselves
  4. Separate these functions between "directable" and "non-directable" (i.e. functions that can go either right or left, or no direction at all6.
  5. Dump these functions within a macro, back at real code.
  6. Of course test this thoroughly. I don;t trust macros that much :sweat_smile: (maybe there;s a way to do it without them :thinking: )
avelino commented 1 month ago

@J0sueTM check this project https://github.com/emlyn/tortilla

J0sueTM commented 1 month ago

@avelino I like the project, and I saw some interesting stuff in the source code. I'm quite sure we can make a small interface for ourselves based on emlyn's tortilla.

With interesting stuff I mean the .getConstructors, .getMethods from Java primitive classes that I forgot existed. Maybe we won't need to do much work after all.

Do you think it would be best to wrap a couple of selected functions for now, and then take some time later to employ this dynamic wrapping mechanism? @Felipe-gsilva I'll be creating an issue and assigning it to you, if that's ok.

Felipe-gsilva commented 1 month ago

@J0sueTM fine

https://github.com/trptr/java-wrapper/blob/master/src/trptr/java_wrapper/locale.clj

this is a small wrapper built for java.util.Locale, but maybe can have some interesting ideas for our use case.

J0sueTM commented 1 month ago

Thanks for the resource @Felipe-gsilva .

this is a small wrapper built for java.util.Locale, but maybe can have some interesting ideas for our use case.

That repo is, however, a really complex wrapper that is way out of bounds from what our usecase. We, as for the current necessities and constraints, just need to get the function name and call itself based on a given client. See the example in my first comment.

J0sueTM commented 1 month ago

@avelino @Felipe-gsilva I was testing around and got pretty close to retrieving every method correctly. Actually I did:

  (import [java.util Arrays])
  (require '[clojure.string :as str])

  (def my-client (create-client "redis://localhost:6379"))

  (->> (.. @my-client getClass getMethods)
       (take 10)
       (map
        (fn [class]
          {:name (.getName class)
           :parameters (map #(.getName %) (.getParameters class))}))
       (clojure.pprint/pprint))

;; =>

  ({:name "pipelined", :parameters ()}
   {:name "pipelined", :parameters ()}
   {:name "getPool", :parameters ()}
   {:name "executeCommand", :parameters ("arg0")}
   {:name "executeCommand", :parameters ("arg0")}
   {:name "broadcastCommand", :parameters ("arg0")}
   {:name "setBroadcastAndRoundRobinConfig", :parameters ("arg0")}
   {:name "ping", :parameters ()}
   {:name "flushDB", :parameters ()}
   {:name "flushAll", :parameters ()})

When compiling a java package, we lose the argnames, so we get these weird looking arg parameters.

This is why I forked jedis, so we can build it ourselvel with the -parameters flag enabled, consequently being able to read these damn parameters.

J0sueTM commented 1 month ago

@avelino @Felipe-gsilva I've finished retrieving every function from the jedis source code and parsing them up. I just need to dump it all into the final source code.

Do you guys think that it would be best to keep the functions inside queue.clj or inside core.clj.

The main pro of putting into core.clj is that not all functions are about queues. And since it will all be generated in the fly, it feels better for the enduser to just include rq.core and use the functions dynamically loaded.

See the current HEAD at feat/dynamic-queue-fns for the source code I'm referring to.

avelino commented 1 month ago

@J0sueTM I like the edn implementation of the functions we need to map with the reflection

I would make a function that receives the "type" (queue or pubsub), so I would have a specific edn for each "type" with the functions we need for it (queue or pubsub).

The function would be called within the namespace we want