Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop >> mail # dev >> Review Request 12774: Fix for SQOOP-1075: Persist framework version in repository


Copy link to this message
-
Re: Review Request 12774: Fix for SQOOP-1075: Persist framework version in repository

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12774/#review24033
-----------------------------------------------------------
Hi Rangav,
thank you very much for updating your patch. I do have additional notes:
common/src/main/java/org/apache/sqoop/common/VersionInfo.java
<https://reviews.apache.org/r/12774/#comment47828>

    The class VersionInfo contains information about the code itself (when it was compiled, by who, from what revision, ...). This information is stored in common module and as a result both client and server will have different values (provided that they are running on different versions). Putting here value specific for core module (e.g. for server) is very confusing as client might see different value then the server itself. I believe that the framework version should be stored inside the FrameworkManager class and that MFramework class should be just a container for it.

common/src/main/java/org/apache/sqoop/model/MFramework.java
<https://reviews.apache.org/r/12774/#comment47829>

    I do not quite understand the semantics of this method? What is the expected usage?
    
    It seems that it's suppose to return most up-to-date version that is available. However the value is taken from static class, so it will return different value on client and server provided that they are running on different versions.

repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
<https://reviews.apache.org/r/12774/#comment47831>

    Nit: Rest of the framework is using property names in dot format, so I would suggest to rename this to "framework.version" rather than using the camel case "frameworkVersion".

repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/12774/#comment47830>

    I'm assuming that there should be call getVersion() instead of getCurrentVersion()?
Jarcec

- Jarek Cecho
On July 22, 2013, 8:03 p.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12774/
> -----------------------------------------------------------
>
> (Updated July 22, 2013, 8:03 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1075
>     https://issues.apache.org/jira/browse/SQOOP-1075
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> A summary of changes:
> 1. MFramework: added fields, getter & setter method for version. Made changes to other methods to take version into account.
> 2. MConnector: since it extends MFramework, can use his version field and methods
> 3. FrameworkBean, FormSerialization: encode/decode version field to json
> 4. DerbyRepositoryHandler, DerbyRepoError: added private methods to persist/fetch framework version; update framework version when registering/updating framework
> 5. TestFrameworkHandling: added test - the test checks for current version of framework, changes it to lower version updates the framework and checks the version of framework once again
> 6. TestSqoopClient, TestUtils, TestMFramework, FrameworkManager, DerbyTestCase: added version parameter to the MFramework constructor call.
>
>
> Diffs
> -----
>
>   common/src/main/java/org/apache/sqoop/common/VersionInfo.java dcf522f
>   common/src/main/java/org/apache/sqoop/json/FrameworkBean.java ad4753b
>   common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java 98768d6
>   common/src/main/java/org/apache/sqoop/model/MConnector.java 1c2c422
>   common/src/main/java/org/apache/sqoop/model/MFramework.java 694f022
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java 607b8d5
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aeb7533
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f717abf