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

Feature/storehaus cassandra only #282

Open
wants to merge 117 commits into
base: develop
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Sep 7, 2015

As discussed in #211

"com.datastax.cassandra" % "cassandra-driver-core" % cassandraDriverVersion classifier "shaded" exclude("io.netty", "*"),
"com.websudos" %% "phantom-dsl" % "1.5.0" exclude ("com.datastax.cassandra", "cassandra-driver-core"),
withCross("com.twitter" %% "util-zk" % utilVersion) exclude ("com.google.guava", "guava"),
bogus210Version("com.chuusai" %% "shapeless" % "2.0.0"),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not 2.2.5? Tests pass just fine on 2.2.5 with a couple of minor changes (that are arguably improvements, anyway).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.2.5 doesn't seem to distinguish between scala 2.10.x any more. is it safe to assume that it will be working with all 2.10s?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndreasPetter It'll work with any 2.10 after 2.10.1.

@ianoc
Copy link
Collaborator

ianoc commented Sep 11, 2015

I think I'd rather we make a storehaus-contrib's repo and move these sorts of stores there. Any upgrades to support new scala versions to the core get blocked on lots of these external stores with questionable external support around them. I'd rather we have a core which we can be very confident about and the contrib repo people can update or not update as they like. I'm afraid of our builds being blocked by say flaky tests in elastic search that we know nothing about the code. Thoughts @johnynek ?

* used to map over an example key to implicit serializers
*/
object cassandraSerializerCreation extends Poly1 {
implicit def default[T : CassandraPrimitive] = at[T](_ => implicitly[CassandraPrimitive[T]])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend including the Case.Aux annotation here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to bother you again, but I'm not able to bring "annotation" in useful context here - I'm not really a shapeless pro. What I do understand is that I use CaseBuilder (and therefore Case) under the hood by using at. I found an annotation.scala in shapeless but as far as I understand (and i must admit that i did not understand too much there) this is for witnessing annotations on types - for which I do not know how to use them here. Maybe you meant that it would be beneficial to add the Case.Aux implicit like implicit c: Case.Aux[T, CassandraPrimtive[T]]?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@travisbrown ah sorry, I just understood that you stated that it would be better to specify the return type with Case.Aux. Forget my previous comment, sorry.

@AndreasPetter
Copy link

Hm... Travis tests are failing, with the note "Killed". Since they run locally and no error shows up it is not unlikely that this has something to do with https://docs.travis-ci.com/user/common-build-problems/#My-build-script-is-killed-without-any-error . How do you arrange tests at Twitter if this happens?

@AndreasPetter
Copy link

O.k. I had to use the shaded cassandra driver (sorry @ianoc, there seem to be dependency conflicts with the driver with the other stores), and use an embedded Cassandra version for testing. Are there any more comments?

@CLAassistant
Copy link

CLAassistant commented Jul 18, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ianoc
❌ AndreasPetter
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

7 participants