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

[ZEPPELIN-559] Cassandra interpreter v2 #600

Closed
wants to merge 9 commits into from

Conversation

doanduyhai
Copy link
Contributor

What is this PR for?

  • Update the Cassandra interpreter to V2 to support new schema commands to describe Cassandra 3.x features:
    • User Defined Functions
    • User Defined Aggregates
    • Materialized Views
  • Add support for single line comment using double slashes (//)
  • Add DESCRIBE TYPES
  • The Java driver version is bumped to 3.0.0-rc1
  • The contextual HELP menu is also updated and a ChangeLog section is added.

What type of PR is it?

[Improvement]

Todos

  • - Add new PR to update official Cassandra interpreter with new features
  • - Test steps executed and confirmed working by the community
  • - Code review by the community (warning, Scala inside)

Is there a relevant Jira issue?

ZEPPELIN-559
ZEPPELIN-575 for doc

How should this be tested?

  1. Download and install locally Cassandra 3.1.1
  2. Start Cassandra
  3. Clone this pull request locally with:
    1. git fetch origin pull/600/head:CassandraInterpreterV2
    2. git checkout CassandraInterpreterV2
  4. Build this version of Zeppelin with mvn clean package -DskipTests
  5. Start Zeppelin and update the property cassandra.hosts of the Cassandra interpreter (set it to localhost or 127.0.0.1 depending on your configuration)
  6. In a paragraph, execute all the CQL statements given in this CassandraInterpreterV2TestData.cql by copy-pasting them
  7. Start a new %cassandra paragraph and execute DESCRIBE FUNCTION test.maxOf;. You should be able to see:
    image
  8. Start a new %cassandra paragraph and execute DESCRIBE AGGREGATE test.group_by;. You should be able to see:
    image
  9. Start a new %cassandra paragraph and execute DESCRIBE MATERIALIZED VIEW test.user_by_country;. You should be able to see:
    image
  10. Start a new %cassandra paragraph and execute SELECT id,val1,val2,maxOf(val1,val2) FROM test.test_max;. You should be able to see:
    image
  11. Start a new %cassandra paragraph and execute SELECT group_by(category,sales_count) FROM test.items;. You should be able to see:
    image
  12. Start a new %cassandra paragraph and execute SELECT * FROM test.user_by_country WHERE country='FR';. You should be able to see:
    image

Questions:

  • Does the licenses files need update? --> No
  • Is there breaking changes for older versions? --> No
  • Does this needs documentation? --> Yes

@doanduyhai doanduyhai changed the title Cassandra interpreter v2 [ZEPPELIN-559] Cassandra interpreter v2 Jan 5, 2016
@corneadoug
Copy link
Contributor

@doanduyhai thanks for the contribution, I know you gave some details in the JIRA issue, but could you update this PR description with our PR template? (https://github.com/apache/incubator-zeppelin/blob/master/CONTRIBUTING.md#creating-a-pull-request)

@victorcouste
Copy link

Just checked the full process, Cassandra interpreter v2 and new CQL scripts, all is ok

testcql

@Leemoonsoo
Copy link
Member

@doanduyhai Great improvement! Somehow last commit didn't trigger CI. Could you trigger CI for last commit?


val parsed = parser.parseAll(parser.queries, queries)

parsed.get should be(List(DescribeTableCmd(Some("ks"),"toto")))
parsed.get should be(List(DescribeFunctionCmd(None,"toto")))

Choose a reason for hiding this comment

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

Are these Options? A more idiomatic way to do this would be
parsed should be Some(List(DescribeFunctionCmd(None,"toto")))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, will update the code to use more idiomatic Scala Option

@doanduyhai doanduyhai force-pushed the CassandraInterpreter-V2 branch 3 times, most recently from 6dd7a82 to 4342c0d Compare January 10, 2016 18:53
@doanduyhai
Copy link
Contributor Author

@corneadoug @Leemoonsoo

Code review has been done by @brianwawok and I pushed some refactoring.

The build succeeded but Travis marked it as failed because : The log length has exceeded the limit of 4 Megabytes (this usually means that test suite is raising the same exception over and over)

Looks like this is a very common issue with Travis, it is reported here: travis-ci/travis-ci#3865

@doanduyhai doanduyhai force-pushed the CassandraInterpreter-V2 branch 4 times, most recently from f8bfc03 to 06a9da5 Compare January 13, 2016 21:40
@doanduyhai
Copy link
Contributor Author

@Leemoonsoo @corneadoug Finally successful build

@corneadoug
Copy link
Contributor

Those headers :)
I don't usually review interpreters, but I will test it out :)

@Leemoonsoo
Copy link
Member

Thanks @doanduyhai for big improvement!
LGTM

@felixcheung
Copy link
Member

LGTM. Any more comments or discussion on this PR?

@Leemoonsoo
Copy link
Member

Merge if there're no more discussions

@felixcheung
Copy link
Member

Tested, very nice interpreter thank you @doanduyhai !
A small amendment: to test this, make sure UDF sandbox is enabled in conf/cassandra.yaml
enable_user_defined_functions: true

Let's merge it

@asfgit asfgit closed this in 11a45e2 Jan 24, 2016
Leemoonsoo pushed a commit to Leemoonsoo/zeppelin that referenced this pull request Feb 4, 2016
### What is this PR for?
Update official documentation for **Cassandra interpreter V2**
Related to PR: apache#600

### What type of PR is it?
[Documentation]

### Is there a relevant Jira issue?
**[ZEPPELIN-575]**

### How should this be tested?
Just check the doc after the build to see if you can see the **chapter 14 Change Log**

### Questions:
* Does the licenses files need update? --> **No**
* Is there breaking changes for older versions? --> **No**
* Does this needs documentation? --> **No**

[ZEPPELIN-575]: https://issues.apache.org/jira/browse/ZEPPELIN-575

Author: DuyHai DOAN <doanduyhai@gmail.com>

Closes apache#604 from doanduyhai/CassandraInterpreterDocumentation and squashes the following commits:

d3f7871 [DuyHai DOAN] Use ZEPPELIN_VERSION variable instead of hard-coding
c05d489 [DuyHai DOAN] Revert commit of doc cleaning PR apache#648
88811ee [DuyHai DOAN] Add Zeppelin version along-side with interpreter version
01716e1 [DuyHai DOAN] Cassandra Interpreter V2 doc
e3dd18b [DuyHai DOAN] [ZEPPELIN-382] Add Documentation for Cassandra interpreter in the doc pages
asfgit pushed a commit that referenced this pull request Feb 13, 2016
### What is this PR for?
Update official documentation for **Cassandra interpreter V2**
Related to PR: #600

### What type of PR is it?
[Documentation]

### Is there a relevant Jira issue?
**[ZEPPELIN-575]**

### How should this be tested?
Just check the doc after the build to see if you can see the **chapter 14 Change Log**

### Questions:
* Does the licenses files need update? --> **No**
* Is there breaking changes for older versions? --> **No**
* Does this needs documentation? --> **No**

[ZEPPELIN-575]: https://issues.apache.org/jira/browse/ZEPPELIN-575

Author: DuyHai DOAN <doanduyhai@gmail.com>

Closes #604 from doanduyhai/CassandraInterpreterDocumentation and squashes the following commits:

b1e70cb [DuyHai DOAN] Remove un-necessary whitespaces
80fcea4 [DuyHai DOAN] Add ZEPPELIN_VERSION in _config.yml
f052bd8 [DuyHai DOAN] Fixes reference to ZEPPELIN_VERSION in markdown
d3f7871 [DuyHai DOAN] Use ZEPPELIN_VERSION variable instead of hard-coding
c05d489 [DuyHai DOAN] Revert commit of doc cleaning PR #648
88811ee [DuyHai DOAN] Add Zeppelin version along-side with interpreter version
01716e1 [DuyHai DOAN] Cassandra Interpreter V2 doc
e3dd18b [DuyHai DOAN] [ZEPPELIN-382] Add Documentation for Cassandra interpreter in the doc pages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants