Skip to content

[ZEPPELIN-2216] general solution to precode. refactoring jdbc precode#2221

Closed
tinkoff-dwh wants to merge 5 commits into
apache:masterfrom
tinkoff-dwh:ZEPPELIN-2216
Closed

[ZEPPELIN-2216] general solution to precode. refactoring jdbc precode#2221
tinkoff-dwh wants to merge 5 commits into
apache:masterfrom
tinkoff-dwh:ZEPPELIN-2216

Conversation

@tinkoff-dwh
Copy link
Copy Markdown
Contributor

What is this PR for?

General solution to execute precode. Refactoring jdbc precode using general solution. Task contains to subtasks: executeAfterOpen, executeBeforeClose. executeBeforeClose not done because we need the context so there is a solution only for executeAfterOpen.

What type of PR is it?

Feature | Refactoring

What is the Jira issue?

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

How should this be tested?

  1. Add parameter zeppelin.PySparkInterpreter.precode someVar='text'
  2. Execute
%pyspark
print(someVar)

Questions:

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

@tinkoff-dwh
Copy link
Copy Markdown
Contributor Author

ready to review

@tinkoff-dwh
Copy link
Copy Markdown
Contributor Author

@Leemoonsoo
prevous discussion #2096

@Leemoonsoo
Copy link
Copy Markdown
Member

Let me take a look in this weekend!

LazyOpenInterpreter lazy = (LazyOpenInterpreter) interpreter;
if (!lazy.isOpen()) {
lazy.open();
result = lazy.executePrecode(context);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't RemoteInterpreterServer.open() also call executePrecode()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it must be called once per instance of the interprete

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Leemoonsoo
works same as the previous jdbc precode.
case:
user: some_user
JDBC interpreter (per user)

  1. there is precode CREATE table IF NOT EXISTS tbl_#{user}
  2. user executes insert into tbm_some_user ...
    • execute precode
    • execute paragraph text
  3. user executes select *from tbm_some_user
    • execute paragraph text

Spark

  1. there is precode var someConst = 12 ...
  2. User executes var result = 1 + someConst
    • execute precode
    • execute paragraph text
  3. User executes var
result2 = 2 + someConts
print(result2)
  • execute only paragraph text

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for explain.

Currently, unittest https://github.com/apache/zeppelin/pull/2221/files#diff-9c5e6fbaa5b23bc0192b4fedc12b2680R445 executes jdbcInterpreter.executePrecode(interpreterContext); manually after open().

Shell we have other unittest that does not call executePrecode() manually? so lazy.executePrecode in InterpreterServer can be verified.

zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java is one place the unittest actually creates RemoteInterpreter process and test against it. This place might be the place you can locate test for this feature.

Copy link
Copy Markdown
Contributor Author

@tinkoff-dwh tinkoff-dwh Apr 24, 2017

Choose a reason for hiding this comment

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

@Leemoonsoo
fix it, if I understand correctly your comment

ci green https://travis-ci.org/tinkoff-dwh/zeppelin/builds/225094590

@tinkoff-dwh tinkoff-dwh reopened this Apr 24, 2017
@Leemoonsoo
Copy link
Copy Markdown
Member

LGTM and merge to master if no further discussions.

@asfgit asfgit closed this in 2c504c4 Apr 28, 2017
@tinkoff-dwh tinkoff-dwh deleted the ZEPPELIN-2216 branch April 28, 2017 10:29
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Sep 1, 2017
### What is this PR for?
General solution to execute precode. Refactoring jdbc precode using general solution. Task contains to subtasks: executeAfterOpen, executeBeforeClose. executeBeforeClose not done because we need the context so there is a solution only for executeAfterOpen.

### What type of PR is it?
Feature | Refactoring

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2216

### How should this be tested?
1. Add parameter zeppelin.PySparkInterpreter.precode `someVar='text'`
2. Execute
```
%pyspark
print(someVar)
```

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

Author: Tinkoff DWH <tinkoff.dwh@gmail.com>

Closes apache#2221 from tinkoff-dwh/ZEPPELIN-2216 and squashes the following commits:

1e3f3f7 [Tinkoff DWH] [ZEPPELIN-2216] fix path
e4cf72f [Tinkoff DWH] [ZEPPELIN-2216] added tests
5a482a0 [Tinkoff DWH] [ZEPPELIN-2216] fix tests
3977722 [Tinkoff DWH] Merge remote-tracking branch 'origin/master' into ZEPPELIN-2216
c0436a2 [Tinkoff DWH] [ZEPPELIN-2216] general solution to precode. refactoring jdbc precode
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Oct 23, 2017
### What is this PR for?
General solution to execute precode. Refactoring jdbc precode using general solution. Task contains to subtasks: executeAfterOpen, executeBeforeClose. executeBeforeClose not done because we need the context so there is a solution only for executeAfterOpen.

### What type of PR is it?
Feature | Refactoring

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2216

### How should this be tested?
1. Add parameter zeppelin.PySparkInterpreter.precode `someVar='text'`
2. Execute
```
%pyspark
print(someVar)
```

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

Author: Tinkoff DWH <tinkoff.dwh@gmail.com>

Closes apache#2221 from tinkoff-dwh/ZEPPELIN-2216 and squashes the following commits:

1e3f3f7 [Tinkoff DWH] [ZEPPELIN-2216] fix path
e4cf72f [Tinkoff DWH] [ZEPPELIN-2216] added tests
5a482a0 [Tinkoff DWH] [ZEPPELIN-2216] fix tests
3977722 [Tinkoff DWH] Merge remote-tracking branch 'origin/master' into ZEPPELIN-2216
c0436a2 [Tinkoff DWH] [ZEPPELIN-2216] general solution to precode. refactoring jdbc precode
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.

2 participants