Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding convenience method to parse Redis version #502

Merged
merged 5 commits into from
Feb 7, 2014
Merged

Adding convenience method to parse Redis version #502

merged 5 commits into from
Feb 7, 2014

Conversation

fabsi88
Copy link
Contributor

@fabsi88 fabsi88 commented Feb 6, 2014

No description provided.

@s-ludwig
Copy link
Member

s-ludwig commented Feb 7, 2014

Thanks! I don't have a Redis installation to test, so I was wondering if all lines of the info string are of the form "key: value" and if it would be safer to iterate over all lines to find the one with "redis_version" instead of assuming the second line. Or does the spec define that the version must be there anyway?

@fabsi88
Copy link
Contributor Author

fabsi88 commented Feb 7, 2014

Np! Referring to the documentation of Redis (http://redis.io/commands/info) it has to be on line 2 in the output of INFO. I also thought about a solution to iterate over all lines but I think this would be some kind of overkill for the version retrievement. Another improvement would be to write a method which returns a specific attribute of the INFO - this would iterate over all lines anyway.

@s-ludwig
Copy link
Member

s-ludwig commented Feb 7, 2014

OK according to the docs, it seems like for a fully robust implementation it would be necessary to also parse the section headers. I think a good future idea would be to change the info() method to return a struct that has a string[string] map for each section instead of the raw string.

Until such a more general facility is there, though, the fixed line method should be fine. The only thing I'd change is to rename getVersion() into @property version() (it could also simply be cached after the connection has been established instead of re-requesting it for every call).

@extrawurst
Copy link
Contributor

@s-ludwig is it even possible to name a method "version" in D ? I just tried it does not built on win32 dmd...

@extrawurst
Copy link
Contributor

and a "Version" method would be violating the dlang style guide. I would suggest calling it redisVersion

@s-ludwig
Copy link
Member

s-ludwig commented Feb 7, 2014

Of course you are right :) The official convention would be to use version_ in this case. Maybe in this case serverVersion?

@extrawurst
Copy link
Contributor

i vote in favor of redisVersion :)

@s-ludwig
Copy link
Member

s-ludwig commented Feb 7, 2014

Considering the original name, I change my vote to redisVersion, too ;)

@s-ludwig
Copy link
Member

s-ludwig commented Feb 7, 2014

Okay, looks good to go! Thanks again!

(that Travic CI error can safely be ignored)

s-ludwig added a commit that referenced this pull request Feb 7, 2014
Adding convenience method to parse Redis version
@s-ludwig s-ludwig merged commit 8fca763 into vibe-d:master Feb 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants