Skip to content

[WIP] ZEPPELIN-1571. Support pig udf interpreter - #1545

Closed
zjffdu wants to merge 5 commits into
apache:masterfrom
zjffdu:ZEPPELIN-1571
Closed

[WIP] ZEPPELIN-1571. Support pig udf interpreter#1545
zjffdu wants to merge 5 commits into
apache:masterfrom
zjffdu:ZEPPELIN-1571

Conversation

@zjffdu

Copy link
Copy Markdown
Contributor

What is this PR for?

Support a new interpreter %pig.udf for Pig Interpreter group. User can write pig udf in this interpreter, and will compile/build this udf to jar which would be registered in pig automatically.

What type of PR is it?

[Feature]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

Unit test is added

Screenshots (if appropriate)

Questions:

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

@zjffdu

Copy link
Copy Markdown
ContributorAuthor

@felixcheung@AhyoungRyu Please help review

Comment threadpig/pom.xml
<zeppelin.pig.localRepo>${basedir}/src/test/resources</zeppelin.pig.localRepo>
</systemPropertyVariables>
</configuration>
</plugin>

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This is for test in PigUDFInterpreterTest


package org.apache.zeppelin.pig;

import com.thoughtworks.qdox.JavaProjectBuilder;

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.

whats the license of this qdox library?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

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.

we need to add that to LICENSE file?

@zjffdu

Copy link
Copy Markdown
ContributorAuthor

@minahlee The syntax highlight for java doesn't work, could you help to check what's I missing here ? Thanks

@minahlee

Copy link
Copy Markdown
Member

js file for java syntax highlight(mode-java.js) is never used before so you will need to import it first.
Here are the files where mode-java.js should be added:
https://github.com/apache/zeppelin/blob/master/zeppelin-web/src/index.html#L123
https://github.com/apache/zeppelin/blob/master/zeppelin-web/bower.json#L51

@zjffdu

Copy link
Copy Markdown
ContributorAuthor

Thanks @minahlee It works now.

LOGGER.debug("pig udf jar is created under " + tmpJarFile.getAbsolutePath());
return tmpJarFile;
} catch (IOException e) {
throw e;

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.

add LOGGER.error

@felixcheung

Copy link
Copy Markdown
Member

2 comments, LGTM otherwise.

@Malian

Copy link
Copy Markdown

@zjffdu What's the status of this pull request ? Can we expect it will be merged soon ? Thank you !

@AhyoungRyu

Copy link
Copy Markdown
Contributor

@zjffdu Sadly there are some conflicts now. Could you resolve them?

@zjffduzjffdu changed the title ZEPPELIN-1571. Support pig udf interpreter[WIP] ZEPPELIN-1571. Support pig udf interpreterMar 5, 2017
@zjffdu

Copy link
Copy Markdown
ContributorAuthor

@AhyoungRyu@felixcheung I changed this PR to WIP. As there's still one issue I need to fix. The issue is that if user change the udf, the class can not been reloading unless restarting the interpreter. I need to fix this classloading issue in pig side first, then I will continue this PR.

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

5 participants

@zjffdu@minahlee@felixcheung@Malian@AhyoungRyu