Skip to content

[MINOR] Refactoring Job and Paragraph - #1793

Closed
jongyoul wants to merge 5 commits into
apache:masterfrom
jongyoul:minor/refactoring-job-paragraph
Closed

[MINOR] Refactoring Job and Paragraph#1793
jongyoul wants to merge 5 commits into
apache:masterfrom
jongyoul:minor/refactoring-job-paragraph

Conversation

@jongyoul

@jongyouljongyoul commented Dec 22, 2016

Copy link
Copy Markdown
Member

What is this PR for?

Job class has only two sub classes. I don't think we need Job abstraction class anymore, then I'll move all function of Job into Paragraph and InterpretJob. Paragraph is used into zeppelin-server and InterpretJob is done by remoteInterpreterServer. I think what we disconnect those two different classes is better to maintain each side of codes. This PR moves two variable - result and results - from Job to Paragraph in order to remove Job class. It also based on ZEPPELIN-1594(#1753), thus all reviewers click the last commit and review it.

What type of PR is it?

[Refactoring]

Todos

  • - Moved these variables into Paragraph

What is the Jira issue?

N/A

How should this be tested?

All things are same as before

Screenshots (if appropriate)

N/A

Questions:

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

@jongyouljongyoul changed the title [MINOR] Refactoring Job and Paragraph[WIP][MINOR] Refactoring Job and ParagraphDec 22, 2016
@jongyoul

jongyoul commented Dec 22, 2016

Copy link
Copy Markdown
MemberAuthor
Failed tests: NotebookTest.testAbortParagraphStatusOnInterpreterRestart:760 expected:<ABORT> but was:<RUNNING>

This test passed in my local and all others passed.

@jongyoul
jongyoulforce-pushed the minor/refactoring-job-paragraph branch from 4eff6a6 to 8de238bCompareDecember 24, 2016 17:36
@jongyoul

Copy link
Copy Markdown
MemberAuthor

I've rebased after merging #1753

@jongyouljongyoul changed the title [WIP][MINOR] Refactoring Job and Paragraph[MINOR] Refactoring Job and ParagraphDec 24, 2016
@jongyoul

Copy link
Copy Markdown
MemberAuthor

Will merge it.

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.

1 participant

@jongyoul