You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Miklos Gergely <mg...@hortonworks.com> on 2018/10/25 19:11:00 UTC
Review Request 69174: Refactor LlapStatusServiceDriver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69174/
-----------------------------------------------------------
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-20807
https://issues.apache.org/jira/browse/HIVE-20807
Repository: hive-git
Description
-------
LlapStatusServiceDriver is the class used to determine if LLAP has started. The following problems should be solved by refactoring:
1. The main class is more than 800 lines long,should be cut into multiple smaller classes.
2. The current design makes it extremely hard to write unit tests.
3. There are some overcomplicated, over-engineered parts of the code.
4. Most of the code is under org.apache.hadoop.hive.llap.cli, but some parts are under org.apache.hadoop.hive.llap.cli.status. The whole program could be moved to the latter.
5. LlapStatusHelpers serves as a class for holding classes, which doesn't make much sense.
This is the first step of refactoring the program, now all of it components are moved under the package org.apache.hadoop.hive.llap.cli.status, all the classes and enums are put into a separate file, the overcomplicated parts of the command line parsing are replaced with a more simple structure, and the findbugs and checkstyle warnings are fixed.
Diffs
-----
bin/ext/llapstatus.sh 2d2c8f4
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java af47b26
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusOptionsProcessor.java dca0c7b
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusServiceDriver.java a521799
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AmInfo.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AppStatusBuilder.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/ExitCode.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapInstance.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusCliException.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusHelpers.java 5c8aeb0
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceDriver.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/State.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/TestLlapStatusServiceDriver.java 54166d5
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/TestLlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
Diff: https://reviews.apache.org/r/69174/diff/1/
Testing
-------
Tested on clusters that
Thanks,
Miklos Gergely
Re: Review Request 69174: Refactor LlapStatusServiceDriver
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69174/
-----------------------------------------------------------
(Updated Nov. 7, 2018, 5 p.m.)
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-20807
https://issues.apache.org/jira/browse/HIVE-20807
Repository: hive-git
Description
-------
LlapStatusServiceDriver is the class used to determine if LLAP has started. The following problems should be solved by refactoring:
1. The main class is more than 800 lines long,should be cut into multiple smaller classes.
2. The current design makes it extremely hard to write unit tests.
3. There are some overcomplicated, over-engineered parts of the code.
4. Most of the code is under org.apache.hadoop.hive.llap.cli, but some parts are under org.apache.hadoop.hive.llap.cli.status. The whole program could be moved to the latter.
5. LlapStatusHelpers serves as a class for holding classes, which doesn't make much sense.
This is the first step of refactoring the program, now all of it components are moved under the package org.apache.hadoop.hive.llap.cli.status, all the classes and enums are put into a separate file, the overcomplicated parts of the command line parsing are replaced with a more simple structure, and the findbugs and checkstyle warnings are fixed.
Diffs (updated)
-----
bin/ext/llapstatus.sh 2d2c8f4
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java af47b26
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusOptionsProcessor.java dca0c7b
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusServiceDriver.java a521799
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AmInfo.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AppStatusBuilder.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/ExitCode.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapInstance.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusCliException.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusHelpers.java 5c8aeb0
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceDriver.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/State.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/TestLlapStatusServiceDriver.java 54166d5
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/TestLlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
service/src/java/org/apache/hive/http/LlapServlet.java 92264d2
Diff: https://reviews.apache.org/r/69174/diff/4/
Changes: https://reviews.apache.org/r/69174/diff/3-4/
Testing
-------
Tested on clusters that
Thanks,
Miklos Gergely
Re: Review Request 69174: Refactor LlapStatusServiceDriver
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69174/
-----------------------------------------------------------
(Updated Oct. 30, 2018, 1:15 a.m.)
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-20807
https://issues.apache.org/jira/browse/HIVE-20807
Repository: hive-git
Description
-------
LlapStatusServiceDriver is the class used to determine if LLAP has started. The following problems should be solved by refactoring:
1. The main class is more than 800 lines long,should be cut into multiple smaller classes.
2. The current design makes it extremely hard to write unit tests.
3. There are some overcomplicated, over-engineered parts of the code.
4. Most of the code is under org.apache.hadoop.hive.llap.cli, but some parts are under org.apache.hadoop.hive.llap.cli.status. The whole program could be moved to the latter.
5. LlapStatusHelpers serves as a class for holding classes, which doesn't make much sense.
This is the first step of refactoring the program, now all of it components are moved under the package org.apache.hadoop.hive.llap.cli.status, all the classes and enums are put into a separate file, the overcomplicated parts of the command line parsing are replaced with a more simple structure, and the findbugs and checkstyle warnings are fixed.
Diffs (updated)
-----
bin/ext/llapstatus.sh 2d2c8f4
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java af47b26
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusOptionsProcessor.java dca0c7b
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusServiceDriver.java a521799
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AmInfo.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AppStatusBuilder.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/ExitCode.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapInstance.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusCliException.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusHelpers.java 5c8aeb0
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceDriver.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/State.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/TestLlapStatusServiceDriver.java 54166d5
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/TestLlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
service/src/java/org/apache/hive/http/LlapServlet.java 92264d2
Diff: https://reviews.apache.org/r/69174/diff/3/
Changes: https://reviews.apache.org/r/69174/diff/2-3/
Testing
-------
Tested on clusters that
Thanks,
Miklos Gergely
Re: Review Request 69174: Refactor LlapStatusServiceDriver
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69174/
-----------------------------------------------------------
(Updated Oct. 25, 2018, 8:57 p.m.)
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-20807
https://issues.apache.org/jira/browse/HIVE-20807
Repository: hive-git
Description
-------
LlapStatusServiceDriver is the class used to determine if LLAP has started. The following problems should be solved by refactoring:
1. The main class is more than 800 lines long,should be cut into multiple smaller classes.
2. The current design makes it extremely hard to write unit tests.
3. There are some overcomplicated, over-engineered parts of the code.
4. Most of the code is under org.apache.hadoop.hive.llap.cli, but some parts are under org.apache.hadoop.hive.llap.cli.status. The whole program could be moved to the latter.
5. LlapStatusHelpers serves as a class for holding classes, which doesn't make much sense.
This is the first step of refactoring the program, now all of it components are moved under the package org.apache.hadoop.hive.llap.cli.status, all the classes and enums are put into a separate file, the overcomplicated parts of the command line parsing are replaced with a more simple structure, and the findbugs and checkstyle warnings are fixed.
Diffs (updated)
-----
bin/ext/llapstatus.sh 2d2c8f4
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java af47b26
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusOptionsProcessor.java dca0c7b
llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusServiceDriver.java a521799
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AmInfo.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/AppStatusBuilder.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/ExitCode.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapInstance.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusCliException.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusHelpers.java 5c8aeb0
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/LlapStatusServiceDriver.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/State.java PRE-CREATION
llap-server/src/java/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/TestLlapStatusServiceDriver.java 54166d5
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/TestLlapStatusServiceCommandLine.java PRE-CREATION
llap-server/src/test/org/apache/hadoop/hive/llap/cli/status/package-info.java PRE-CREATION
Diff: https://reviews.apache.org/r/69174/diff/2/
Changes: https://reviews.apache.org/r/69174/diff/1-2/
Testing
-------
Tested on clusters that
Thanks,
Miklos Gergely