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 914]- Apply new mechanism to IgniteInterpreter #1561

Closed
wants to merge 8 commits into from

Conversation

meenakshisekar
Copy link
Contributor

What is this PR for?

This handles replacing the registration of interpreter with static block by the interpreter-setting.json file

What type of PR is it?

[| Improvement |

Todos

Sub-Task

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-914

How should this be tested?

There shouldn't be any warning like below on starting the server
INFO 2016-09-29 00:25:46,247 - Bootstrapping Ignite Interpreter WARN 2016-09-29 00:25:46,250 - Static initialization is deprecated for interpreter Ignite, You should change it to use interpreter-setting.json in your jar or interpreter/{interpreter}/interpreter-setting.json INFO 2016-09-29 00:25:46,250 - Inclass=org.apache.zeppelin.ignite

And ensure that the Ignite related paragraphs run without any error

Screenshots (if appropriate)

Questions:

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

"ignite.addresses": {
"envName": null,
"propertyName": "ignite.addresses",
"defaultValue": "127.0.0.1:47500..47509,Coma separated list of addresses",
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems there is a typo in here: Coma -> Comma

Copy link
Contributor

@vectorijk vectorijk Oct 25, 2016

Choose a reason for hiding this comment

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

"Comma separated list of addresses" should be in description below instead of in the defaultValue

Copy link
Contributor

Choose a reason for hiding this comment

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

@vectorijk right good catch :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@meenakshisekar CI failure may caused by this. Could you address comment we gave?

@meenakshisekar
Copy link
Contributor Author

Reopening for travis build

@meenakshisekar
Copy link
Contributor Author

Reopening for Travis build.

@meenakshisekar meenakshisekar changed the title Zeppelin 914 Apply new mechanism to IgniteInterpreter [Zeppelin 914]- Apply new mechanism to IgniteInterpreter Oct 31, 2016
@meenakshisekar
Copy link
Contributor Author

Reopening for TRavis

@jongyoul
Copy link
Member

jongyoul commented Nov 2, 2016

@meenakshisekar The failures of this PR is related to actual behavior, not flaky. Can you take a look into the result of the test?

@bzz
Copy link
Member

bzz commented Dec 5, 2016

@meenakshisekar thank you for contribution! Could you rebase on the latest master please?

Also, please do not hesitate to let us know in case you need any help with this one, in order to get it merge!

@jongyoul
Copy link
Member

@meenakshisekar Could you rebase this PR?

@DrIgor
Copy link
Contributor

DrIgor commented Dec 28, 2016

Hello!

I would like to help with ZEPPELIN-804 subtasks and make it possible to complete refactoring

If you don't mind I will rebase this PR and take a look at #1077

@meenakshisekar
Copy link
Contributor Author

meenakshisekar commented Dec 28, 2016 via email

@DrIgor
Copy link
Contributor

DrIgor commented Dec 29, 2016

New pull request #1819

I rebased it on the latest master and fixed failing test

@jongyoul
Copy link
Member

jongyoul commented Jan 2, 2017

@meenakshisekar can you please close this PR?

@asfgit asfgit closed this in c38a0a0 May 9, 2018
asfgit pushed a commit that referenced this pull request May 9, 2018
close #83
close #86
close #125
close #133
close #139
close #146
close #193
close #203
close #246
close #262
close #264
close #273
close #291
close #299
close #320
close #347
close #389
close #413
close #423
close #543
close #560
close #658
close #670
close #728
close #765
close #777
close #782
close #783
close #812
close #822
close #841
close #843
close #878
close #884
close #918
close #989
close #1076
close #1135
close #1187
close #1231
close #1304
close #1316
close #1361
close #1385
close #1390
close #1414
close #1422
close #1425
close #1447
close #1458
close #1466
close #1485
close #1492
close #1495
close #1497
close #1536
close #1545
close #1561
close #1577
close #1600
close #1603
close #1678
close #1695
close #1739
close #1748
close #1765
close #1767
close #1776
close #1783
close #1799
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.

6 participants