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

initial clean up of Astyanax ColumnFamily #646

Merged
merged 4 commits into from Mar 8, 2016

Conversation

@shintasmith
Copy link
Contributor

@shintasmith shintasmith commented Mar 7, 2016

Why
Eventually, we will be moving to using Datastax driver and removing Astyanax, in preparation to upgrading to Cassandra 2.1.13.

This PR removes some unnecessary use of Astyanax's ColumnFamily model class. Blueflood already provides a wrapper class called MetricColumnFamily, which extends the Astyanax ColumnFamily class. But for some reasons some code are written to return Astyanax ColumnFamily directly, propagating the use of this class in more places than necessary.

How

  • Change the CassandraModel.getColumnFamily() method to return MetricColumnFamily.
  • Change all consumer of this method accordingly.

A small other clean up I did was to move the class IntegrationBaseTest.java from src/main to src/integration-test. To make this move successful, I have to also change .travis.yml. Before the PR, we were calling mvn package before running the tests. I think that is incorrect. We should package after tests are running successfully.

I have run the unit & integration tests and they all have passed.

@ChandraAddala
Copy link
Contributor

@ChandraAddala ChandraAddala commented Mar 7, 2016

Tests seem to be failing. Other than that everything looks good.

@shintasmith
Copy link
Contributor Author

@shintasmith shintasmith commented Mar 7, 2016

Yeah... I'm working on it. One of the base class IntegrationBaseTest.java was placed in src/main. I thought I would move it to the right place, which is src/integration-test. But Travis is not happy yet..

@shintasmith
Copy link
Contributor Author

@shintasmith shintasmith commented Mar 8, 2016

@usnavi
Copy link
Contributor

@usnavi usnavi commented Mar 8, 2016

lgtm

@ChandraAddala
Copy link
Contributor

@ChandraAddala ChandraAddala commented Mar 8, 2016

Looks good.

@shintasmith
Copy link
Contributor Author

@shintasmith shintasmith commented Mar 8, 2016

thanks all

shintasmith added a commit that referenced this pull request Mar 8, 2016
initial clean up of Astyanax ColumnFamily
@shintasmith shintasmith merged commit 2d4972e into rackerlabs:master Mar 8, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@izrik
Copy link
Contributor

@izrik izrik commented Mar 10, 2016

@shintasmith I've run into this problem before. I found that the integration-test phase comes after the package phase. Integration tests can't be run before then. More info here: https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html#Lifecycle_Reference

@shintasmith
Copy link
Contributor Author

@shintasmith shintasmith commented Mar 10, 2016

@izrik, I have fixed this

@coveralls
Copy link

@coveralls coveralls commented Mar 12, 2018

Coverage Status

Coverage decreased (-30.4%) to 42.727% when pulling 34f7547 on shintasmith:driver_premigration into 9795c45 on rackerlabs:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants