> On Oct. 8, 2014, 5:31 p.m., Gwen Shapira wrote: > > core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java, line > > 140 > > <https://reviews.apache.org/r/26419/diff/3/?file=715511#file715511line140> > > > > I think the name should indicate that this is connector config, not a > > connector object. > > Veena Basavaraj wrote: > its a configurable rather than config. > > Gwen Shapira wrote: > Connector is a configurable, but MConnector contains configs - > linkConfig, fromConfig, toConfig. Therefore what we are setting is basically > a bunch of configs relavant to the connector. > > Veena Basavaraj wrote: > Am I missing something here, the MConnector is a object that represents > the connecto class, its name, its version and the config objects it provides. > so it is not just the configs.
MConnector reperesents the connector metadata (confs, name and version as you noted). There is a different Connector class. If we leave the name getConnector, it sounds like we are returning a Connector object (not just the metadata object). I got seriously confused connector object vs. connector metadata object when writing the dump/load utils. I think the new naming will increase confusion rather than decrease it. Therefore a name that will clarify that this method only returns the metadata will be appreciated. - Gwen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26419/#review55826 ----------------------------------------------------------- On Oct. 8, 2014, 7:50 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26419/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2014, 7:50 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-SQOOP-1367 > > > Description > ------- > > Few more pending renames as per the agreement on Sqoop 1551 > > - Mainly fix the upgrade logic for 1498 changes > - rename the repository upgrader to Configurable upgrader - agreed by Jarcec. > > - rename configuration utils to MRConfigurationUtils, so it is not be > confused with the sqoop configuration/ configs > - rename mapreduce to MR ( if this is not acceptable, happy to change it back) > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/model/ConfigUtils.java 290e7fc > common/src/main/java/org/apache/sqoop/model/MConfigurable.java PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MConfigurableType.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191 > common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e > common/src/main/java/org/apache/sqoop/utils/ClassUtils.java 0be4d41 > common/src/main/java/org/apache/sqoop/validation/ConfigValidator.java > eac789e > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java > 87ac2af > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorUpgrader.java > a069b3e > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConfigUpgrader.java > b17aa21 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > 606b9fa > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnectorUpgrader.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/connector/ConnectorError.java d544fb1 > core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java 54bdd13 > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5226926 > core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb > core/src/main/java/org/apache/sqoop/driver/DriverConfigUpgrader.java > 847b73d > core/src/main/java/org/apache/sqoop/driver/DriverUpgrader.java PRE-CREATION > core/src/main/java/org/apache/sqoop/driver/JobManager.java df2a5ab > core/src/main/java/org/apache/sqoop/driver/JobRequest.java 2666320 > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 3ade247 > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > 97de893 > core/src/main/java/org/apache/sqoop/repository/Repository.java 95c7a4d > core/src/main/java/org/apache/sqoop/repository/RepositoryManager.java > c2f8505 > core/src/test/java/org/apache/sqoop/driver/TestDriverConfigUpgrader.java > dc4e8c8 > core/src/test/java/org/apache/sqoop/driver/TestDriverUpgrader.java > PRE-CREATION > core/src/test/java/org/apache/sqoop/driver/TestJobManager.java 3b475c6 > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java > e6e4760 > > execution/mapreduce/src/main/java/org/apache/sqoop/execution/mapreduce/MapreduceExecutionEngine.java > 47f8478 > execution/mapreduce/src/main/java/org/apache/sqoop/job/JobConstants.java > 349bb60 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/MRExecutionError.java > PRE-CREATION > execution/mapreduce/src/main/java/org/apache/sqoop/job/MRJobConstants.java > PRE-CREATION > > execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java > 1dc12d1 > execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 5423b7b > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/ConfigurationUtils.java > 0fa07f7 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/MRConfigurationUtils.java > PRE-CREATION > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopDestroyerExecutor.java > 8d2a1da > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopFileOutputFormat.java > ca77e16 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopInputFormat.java > 1c1133a > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java > 03d84d4 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopNullOutputFormat.java > 594b5e9 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java > 1ebd3e4 > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopReducer.java > a55534a > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java > dca4c90 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java > e3b68e2 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java > 7f9a147 > > execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java > f5742a2 > > execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestConfigurationUtils.java > 501e32c > > execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestMRConfigurationUtils.java > PRE-CREATION > > execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java > 1f411d2 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java > 74e41df > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 73d8387 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > ad749ed > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > 478afe2 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > 2da084f > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestDriverHandling.java > d597bd8 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInternals.java > 0eb9df4 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestRespositorySchemaUpgrade.java > PRE-CREATION > server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java > 7109ae5 > > server/src/main/java/org/apache/sqoop/handler/DriverConfigRequestHandler.java > aa773a9 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > 462579c > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 80e65b8 > shell/src/main/java/org/apache/sqoop/shell/SqoopCommand.java cbd34f5 > spi/src/main/java/org/apache/sqoop/connector/spi/ConfigurableUpgrader.java > PRE-CREATION > spi/src/main/java/org/apache/sqoop/connector/spi/RepositoryUpgrader.java > 879e428 > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java > 849f091 > > submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java > 0c492ef > > Diff: https://reviews.apache.org/r/26419/diff/ > > > Testing > ------- > > yes test pass > > > Thanks, > > Veena Basavaraj > >
