ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yusaku Sako" <yus...@hortonworks.com>
Subject Re: Review Request 39980: Hive View : Add Database/Table creation from File
Date Fri, 06 Nov 2015 15:29:51 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39980/#review105433
-----------------------------------------------------------



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/client/Row.java (line 52)
<https://reviews.apache.org/r/39980/#comment164031>

    Indentation looks off compared to the code above.



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/ColumnDescriptionImpl.java
(line 26)
<https://reviews.apache.org/r/39980/#comment164032>

    remove



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/DataParser.java
(line 29)
<https://reviews.apache.org/r/39980/#comment164033>

    remove



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/IParser.java
(line 30)
<https://reviews.apache.org/r/39980/#comment164034>

    remove



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/ParseOptions.java
(line 24)
<https://reviews.apache.org/r/39980/#comment164035>

    remove



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/ParseUtils.java
(line 28)
<https://reviews.apache.org/r/39980/#comment164036>

    remove



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/TableInfo.java
(line 26)
<https://reviews.apache.org/r/39980/#comment164037>

    remove



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/UploadService.java
(line 261)
<https://reviews.apache.org/r/39980/#comment164029>

    "if" split across multiple lines without braces  is error prone / bad practice.



contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/UploadService.java
(line 264)
<https://reviews.apache.org/r/39980/#comment164030>

    Remove if not used or comment.



contrib/views/hive/src/main/resources/ui/hive-web/app/controllers/upload-table.js (line 191)
<https://reviews.apache.org/r/39980/#comment164023>

    remove



contrib/views/hive/src/main/resources/ui/hive-web/app/controllers/upload-table.js (line 210)
<https://reviews.apache.org/r/39980/#comment164025>

    remove



contrib/views/hive/src/test/java/org/apache/ambari/view/hive/resources/upload/DataParserTest.java
(line 35)
<https://reviews.apache.org/r/39980/#comment164026>

    remove



contrib/views/hive/src/test/java/org/apache/ambari/view/hive/resources/upload/DataParserTest.java
(line 63)
<https://reviews.apache.org/r/39980/#comment164027>

    If we are going to leave this with TODO, please give a more meaningful description.


In addition, I noticed general inconsistencies in indentations (4 spaces rather than 2 in
new files) and style.
>From Coding Guidelines (https://cwiki.apache.org/confluence/display/AMBARI/Coding+Guidelines+for+Ambari)
"Code must be formatted according to Sun's conventions, with one exception: Indent two spaces
per level, not four."

- Yusaku Sako


On Nov. 5, 2015, 5:37 p.m., Nitiraj Rathore wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39980/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2015, 5:37 p.m.)
> 
> 
> Review request for Ambari, Srimanth Gunturi, Sid Wagle, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-13747
>     https://issues.apache.org/jira/browse/AMBARI-13747
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Hive View should have a widget to:
> Create/Choose a database and create a table inside it
> Create table using a delimited (such as CSV) file from HDFS or local file system
> Discover column names if it has a header
> Discover suggested data types
> Create a table with a given name
> Optionally create an internal table that is ORC by default
> 
> Implementation Steps :
> User uploads a CSV file. 
> Server reads the input stream and without saving parses it to generate preview rows and
detect datatypes of columns
> Server sends table info to the UI.
> User can change column names and datatypes and submits table info.
> Server creates a table using hive job, and returns job info back.
> UI keeps looping for job success.
> On Job success UI sends the same file again.
> If required the file’s columns are extracted and rest of the input stream is given
to the HDFS Api for uploading to the hive metastore.
> Server returns success or exception.
> 
> Changes : 
> ColumnDescription : added enum for hive data types.?
> ConnectionFactory : Added HdfsApi for connection to HDFS.
> package.json : added dependency ember-cli-uploader
> bower.json : added dependency ember-uploader": "0.3.9"
> view.xml : added resource upload for UploadService
> 
> Additions :
>   resources/uploads: new packages for upload related classes
> 	ColumnDescriptionImpl : another impl ColumnDescription for better handling of data
> 	IParser : Interface for parsers
> 	DataParser : Decorator Parser for hiding internal impls.
> 	CSVParser : for parsing CSV using apache commons-csv
> 	QueryGenerator : class for generating HQL query from given inputs.
> 	UploadService : REST interface for front end
> 	
> 	DataParserTest : the test case class for DataParser.
> 
> 	UI
> 	 router.js : added route /upload-table
> 
> 	/app/adapters : 
> 		file-upload : ember uploader customized
> 		upload-table : adapter for REST calls
> 	
> 	/controllers : 
> 		 upload-table : controller for handling all activities in Upload Table tab
> 		
> 	/templates : 
> 		upload-table : template for UI on Upload Tables tab
> 
> Minor changes :
> 	Row : added equals and hashcode toString
> 	ParseOptions : Parsing options to be passed to parser
> 	ParseUtils : Utility class
> 	TableInfo : Input to QueryGenerator’s method
>   UI
> 	file-upload
> 	navbar-widget : now includes one more tab for upload table
> 	i18n.js : added constants
> 	app.scss : minor change
> 	constants.js : added constants
> 
> 
> Diffs
> -----
> 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/client/ColumnDescription.java
d7ea560 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/client/ConnectionFactory.java
82ac1f5 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/client/Row.java 306530a

>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/CSVParser.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/ColumnDescriptionImpl.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/DataParser.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/IParser.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/ParseOptions.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/ParseUtils.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/QueryGenerator.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/TableInfo.java
PRE-CREATION 
>   contrib/views/hive/src/main/java/org/apache/ambari/view/hive/resources/uploads/UploadService.java
PRE-CREATION 
>   contrib/views/hive/src/main/resources/ui/hive-web/app/adapters/file-upload.js PRE-CREATION

>   contrib/views/hive/src/main/resources/ui/hive-web/app/adapters/upload-table.js PRE-CREATION

>   contrib/views/hive/src/main/resources/ui/hive-web/app/components/file-upload.js PRE-CREATION

>   contrib/views/hive/src/main/resources/ui/hive-web/app/components/navbar-widget.js c3659cf

>   contrib/views/hive/src/main/resources/ui/hive-web/app/controllers/upload-table.js PRE-CREATION

>   contrib/views/hive/src/main/resources/ui/hive-web/app/initializers/i18n.js 5ae9b7e

>   contrib/views/hive/src/main/resources/ui/hive-web/app/router.js 5a51b11 
>   contrib/views/hive/src/main/resources/ui/hive-web/app/styles/app.scss bf271c4 
>   contrib/views/hive/src/main/resources/ui/hive-web/app/templates/upload-table.hbs PRE-CREATION

>   contrib/views/hive/src/main/resources/ui/hive-web/app/utils/constants.js a349d51 
>   contrib/views/hive/src/main/resources/ui/hive-web/bower.json d43881f 
>   contrib/views/hive/src/main/resources/ui/hive-web/package.json 6ecdcb6 
>   contrib/views/hive/src/main/resources/view.xml fdc32d7 
>   contrib/views/hive/src/test/java/org/apache/ambari/view/hive/resources/upload/DataParserTest.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39980/diff/
> 
> 
> Testing
> -------
> 
> DataParserTest added for testing the functionality of DataParser class.
> Manual testing of overall feature, including file uploads in HDFS.
> 
> 
> Thanks,
> 
> Nitiraj Rathore
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message