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

Already on GitHub? Sign in to your account

Being able to toggle constancy level during execute statement #67

Merged
merged 5 commits into from May 15, 2014

Conversation

Projects
None yet
4 participants
Contributor

astronomy88 commented Feb 15, 2014

Would like to have flexibility to configure the consistency level during CQL execute.

For instance, in our use case we have the consistency level set at LOCAL_QUORUM for writes, and ONE for reads.If a user doesn't bother with consistency levels, the default option is QUORUM (which would be the setting that would occur before this change)

@kreynolds kreynolds commented on an outdated diff Feb 18, 2014

lib/cassandra-cql/database.rb
@@ -100,8 +100,12 @@ def prepare(statement, options={}, &block)
end
end
- def execute(statement, *bind_vars)
- result = statement_class.new(self, statement).execute(bind_vars)
+ def execute(statement, consistency=CassandraCQL::Thrift::ConsistencyLevel::QUORUM, *bind_vars)
@kreynolds

kreynolds Feb 18, 2014

Owner

This won't work .. this is a breaking change for anybody who has already been using this library. I think our only option to have this be backwards compatible is to instead have a separate method, something like execute_with_consistency() that takes consistency, statement, *bind_vars.

@kreynolds kreynolds commented on an outdated diff Feb 18, 2014

lib/cassandra-cql/database.rb
@@ -111,9 +115,9 @@ def execute(statement, *bind_vars)
raise Error::InvalidRequestException.new($!.why)
end
- def execute_cql_query(cql, compression=CassandraCQL::Thrift::Compression::NONE)
+ def execute_cql_query(cql, compression=CassandraCQL::Thrift::Compression::NONE, consistency)
@kreynolds

kreynolds Feb 18, 2014

Owner

consistency should have a default option of QUORUM to be backwards compatible. I know its specified in one of the callers, but it should be here as well in case somebody was relying on that behavior

@kreynolds kreynolds commented on an outdated diff Feb 18, 2014

lib/cassandra-cql/statement.rb
@@ -36,7 +36,7 @@ def prepare(statement)
@statement = statement
end
- def execute(bind_vars=[], options={})
+ def execute(bind_vars=[], consistency, options={})
@kreynolds

kreynolds Feb 18, 2014

Owner

consistency should be an option here, not another argument to the method

Owner

kreynolds commented Feb 18, 2014

And its a pain I know, but I'll need tests for the changes as well. I appreciate the effort though, almost there!

Contributor

astronomy88 commented Feb 23, 2014

Hey Kelley - tried to follow along with the suggestions! Thanks for taking the time to go through it!

Contributor

codekitchen commented May 14, 2014

@astronomy88 do you think you'll be writing the tests for this change soon? if not I may take a stab at it, having this in the gem is the last thing we need before we can upgrade our cassandra clusters.

Contributor

astronomy88 commented May 14, 2014

@codekitchen Sorry, looks like my schedule will be a little hectic that next few days. If you don't mind taking a stab at it, that would be great! Otherwise, I might be able to try in a couple weeks!

Contributor

codekitchen commented May 15, 2014

No worries, thanks for doing the bulk of the work. I can't amend this pull request so I started a new one at #71

@kreynolds kreynolds merged commit 9326076 into kreynolds:master May 15, 2014

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment