socketry / async-redis

MIT License
83 stars 18 forks source link

DSL complete? #1

Closed ioquatix closed 5 years ago

ioquatix commented 5 years ago

I merged the DSL branch into master.

Is there more that should be implemented?

aemadrid commented 5 years ago

would love to see the streams (XADD, XRANGE,XGROUP, etc) commands implemented. I've been testing that out and was able to run most of that with plain calls but I'd love to see a proper implementation.

ioquatix commented 5 years ago

@aemadrid Can you show me how you used them? Just so I see what the API should look like.

ioquatix commented 5 years ago

@huba I renamed DSL -> Methods. I removed the unimplemented ones and released 0.3.0 - it seems like all specs are passing.

https://github.com/socketry/async-redis/commit/0f40bba07ddd77e083bc552d63204a7dbfb13bbe

ioquatix commented 5 years ago

I actually didn't write any spec for bitcount.

ioquatix commented 5 years ago

@aemadrid feel free to make a PR with those methods.

huba commented 5 years ago

@ioquatix There is loads more to be implemented, I'm about a quarter of the way through. Mostly it's just simple as wrapping the call function into convenient methods, sometimes a bit of logic is required to make the methods work nicely. I'm basically just systematically going over the groups and implementing everything from the redis protocol spec.

The way it's organized is that each module contains the redis commands from the corresponding groups, the groupings somehow reflect what the commands operate on.

Edit: with that said some of the groups don't make sense in all contexts like the ones related to pub/sub. Which is why it's nice to separate them out like so.

ioquatix commented 5 years ago

That makes sense.

Maybe we can generate the methods from the documentation?

huba commented 5 years ago

would love to see the streams (XADD, XRANGE,XGROUP, etc) commands implemented.

@aemadrid DSL for commands related to streams are definitely in the pipeline.

huba commented 5 years ago

Maybe we can generate the methods from the documentation?

Hmm, I suppose there is no reason why something like XADD key ID field string [field string ...] couldn't be parsed and turned into a ruby method. Just need to extract that from the DOM, but that looks easy enough.

ioquatix commented 5 years ago

Is there something better, e.g. XML from the Redis source code or something?

huba commented 5 years ago

Is there something better, e.g. XML from the Redis source code or something?

How about this? I found this link in a ruby script in the redis source repo.

ioquatix commented 5 years ago

That looks perfect. We could make a task to update the method stubs from that. It could generate the documentation too.

huba commented 5 years ago

I suppose the next question is whether or not we want to make the DSL methods more convenient like I did with SET.

ioquatix commented 5 years ago

I think in the first instance we should just try to generate everything automatically. We could add convenience methods on top some how, or augment the automatically generated version.

huba commented 5 years ago

Hmm, putting wrappers on top of wrappers sounds like a terrible idea. We could have maybe a Methods::StringsAuto module that gets included alongside the Methods::Strings module. The latter having the augmented methods and the tool we used to automatically generate the methods would skip generating the ones that appear in the module with the augmented methods.

ioquatix commented 5 years ago

Yeah, anything is possible at this point. I'd just focus on auto-generation for now.

huba commented 5 years ago

I suppose we still want git to track the auto-generated stuff.

huba commented 5 years ago

Righto, I had a look at generating the DSL f9facedda22450b1aa6c35c3dc6b141275ecc776. That's right a thousand lines of pure awesomeness. It's not doing a very smart job at the moment, it creates variadic methods that simply wrap call. Of course it'll need to do better, but the arguments descriptions in that JSON file are edge case hell. If worst comes to worst this could just be used to generate all the method and then fill in the blanks manually.

ioquatix commented 5 years ago

That's awesome.

ioquatix commented 5 years ago

We moved all of this into protocol-redis.

The DSL is non-trivial but I think we've exposed most of the useful parts.

The next step is to expose some useful high-level data structures, but it makes sense to do this using the DSL, e.g.

class Queue
    def initialize(context, key)
        @context = context
        @key = key
    end

    def pop
        @context.lpop @key
    end
end

To me this is the most useful approach.