Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Multiversion #13

Merged
merged 11 commits into from

2 participants

@thobbs
Collaborator

This allows for Telephus to work with a cluster that has 0.7 or 0.8 nodes (or a mix).

I primarily tried to preserve backwards-compatibility. The main change is that check_api_version has been replaced with an api_version parameter that allows you to skip the version matching. telephus.cassandra.constants has also changed to make this work nicely.

Feel free to make any stylistic changes you want.

@thobbs
Collaborator

Any progress on pulling this? :)

@thepaul

Now wait, where does this 20.1.0 come from? The current VERSION in the cassandra-0.8 branch and the cassandra-0.8.0-beta tag, and even trunk, all say 19.10.0. At least on the git mirror. Is that out of whack or something?

Collaborator

Ah, good catch -- 0.8 was on API version 20.1.0 for a couple of weeks before it was changed and moved back to 19.10.0 for beta1. The thrift definitions are all from the 19.10.0 version, but the version number here never got updated to match.

Collaborator

Oh, it is updated in the pull request -- this is just from the old commit.

Collaborator

My bad. Thought I checked that.

@thepaul

This is a lot of manual assignment that looks like it could be a bit error-prone for maintenance. Is there some reason not to do "from c08.ttypes import *"?

Collaborator

No, there was a reason for them to be explicitly listed with my first approach, but they could be replaced by a 'from c08.ttypes import *".

I'm not familiar with the particulars of imports in python, but I've read that 'from foo import *" is generally bad practice and should be avoided where possible. I figured a library is a reasonable place to be safe like this, but if there's no reason for concern, I'll go ahead and swap it out.

Collaborator

It's bad practice to use it by default, or use it when you don't need it, but this is one of the cases ("making module A inherit from or act like module B") where it's the only sane thing to do.

@thepaul thepaul referenced this pull request from a commit
@thepaul thepaul Merged pull request #13 from thobbs/multiversion.
Multiversion
81f1892
@thepaul thepaul merged commit 81f1892 into driftx:unstable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.