----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12713/#review24060 -----------------------------------------------------------
Hi Mengwei, thank you very much for incorporating my feedback. I do have additional couple of notes: common/src/main/java/org/apache/sqoop/common/EventContext.java <https://reviews.apache.org/r/12713/#comment47903> This class seems to be strictly related to server component and as a result should not be available in the common module that is shared between client and server. I would suggest to create a new package "org.apache.sqoop.request" for it in module "core". common/src/main/java/org/apache/sqoop/common/EventContext.java <https://reviews.apache.org/r/12713/#comment47902> I believe that the EventContext do not need to be serialized anywhere and the access to each item should be as fast as possible as it will be done in each request. As a result I would suggest to use a normal properties rather then the Map. I believe that this context do not even need to extend the MutableMapContext class due to it's very specific use case. Can we also rename the class a bit to more descriptive about the usage? Perhaps HttpEventContext or something similar. Jarcec - Jarek Cecho On July 24, 2013, 5:55 p.m., Mengwei Ding wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12713/ > ----------------------------------------------------------- > > (Updated July 24, 2013, 5:55 p.m.) > > > Review request for Sqoop, Abraham Elmahrek, Hari Shreedharan, and Jarek Cecho. > > > Bugs: SQOOP-1142 > https://issues.apache.org/jira/browse/SQOOP-1142 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit ec9fff7a8f45632e25f0138eb1991007a5967b06 > Author: Mengwei Ding <[email protected]> > Date: Wed Jul 17 10:48:05 2013 -0700 > > SQOOP-1142 Sqoop2: Provide creater and last edited by to metadata > structures > > :100644 100644 b7b0436... e2dc8dc... M > client/src/main/java/org/apache/sqoop/client/core/Constants.java > :100644 100644 32bca71... 08d0a70... M > client/src/main/java/org/apache/sqoop/client/shell/ShowConnectionFunction.java > :100644 100644 590e4e7... 7b8b43e... M > client/src/main/java/org/apache/sqoop/client/shell/ShowJobFunction.java > :100644 100644 cbc956d... 10b3130... M > client/src/main/java/org/apache/sqoop/client/utils/SubmissionDisplayer.java > :100644 100644 5537a8e... d9cc029... M > client/src/main/resources/client-resource.properties > :000000 100644 0000000... 3c5c003... A > common/src/main/java/org/apache/sqoop/common/EventContext.java > :100644 100644 b4e986a... 61cbf7c... M > common/src/main/java/org/apache/sqoop/json/ConnectionBean.java > :100644 100644 a830646... 8baea71... M > common/src/main/java/org/apache/sqoop/json/JobBean.java > :100644 100644 79490f8... 61d6576... M > common/src/main/java/org/apache/sqoop/json/SubmissionBean.java > :100644 100644 98768d6... ea0f71f... M > common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java > :100644 100644 137e71c... 6bbbed5... M > common/src/main/java/org/apache/sqoop/model/MAccountableEntity.java > :100644 100644 dd1d75b... 7aa6356... M > common/src/test/java/org/apache/sqoop/json/TestConnectionBean.java > :100644 100644 3b56171... d87655e... M > common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java > :100644 100644 4ea42b1... 9d1c622... M > common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java > :100644 100644 9f09982... fcef47c... M > core/src/main/java/org/apache/sqoop/framework/JobManager.java > :100644 100644 f717abf... a510a6d... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > :100644 100644 68cb1c0... 85b09ce... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > :100644 100644 b2cd6cc... eb0b62f... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > :100644 100644 677b0be... 40bf384... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > :100644 100644 038f602... 04c248b... M > server/src/main/java/org/apache/sqoop/handler/ConnectionRequestHandler.java > :100644 100644 ab3f9d0... 7bec139... M > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > :100644 100644 65686a8... 49e45b2... M > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/core/Constants.java b7b0436 > > client/src/main/java/org/apache/sqoop/client/shell/ShowConnectionFunction.java > 32bca71 > client/src/main/java/org/apache/sqoop/client/shell/ShowJobFunction.java > 590e4e7 > client/src/main/java/org/apache/sqoop/client/utils/SubmissionDisplayer.java > cbc956d > client/src/main/resources/client-resource.properties 5537a8e > common/src/main/java/org/apache/sqoop/common/EventContext.java PRE-CREATION > common/src/main/java/org/apache/sqoop/json/ConnectionBean.java b4e986a > common/src/main/java/org/apache/sqoop/json/JobBean.java a830646 > common/src/main/java/org/apache/sqoop/json/SubmissionBean.java 79490f8 > common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java > 98768d6 > common/src/main/java/org/apache/sqoop/model/MAccountableEntity.java 137e71c > common/src/test/java/org/apache/sqoop/json/TestConnectionBean.java dd1d75b > common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java 3b56171 > common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java > 4ea42b1 > core/src/main/java/org/apache/sqoop/framework/JobManager.java 9f09982 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > f717abf > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > 68cb1c0 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > b2cd6cc > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > 677b0be > server/src/main/java/org/apache/sqoop/handler/ConnectionRequestHandler.java > 038f602 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > ab3f9d0 > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > 65686a8 > > Diff: https://reviews.apache.org/r/12713/diff/ > > > Testing > ------- > > Unit tests for 'common' module passed. I also did several manual tests to > check the new functionalities. > > > Thanks, > > Mengwei Ding > >
