sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 25148: SQOOP-1468 Sqoop2: Validations: Introduce advanced validators that can be parametrized
Date Wed, 03 Sep 2014 07:26:43 GMT

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

(Updated Sept. 3, 2014, 7:26 a.m.)


Review request for Sqoop.


Changes
-------

Fixed test case per Abe's feedback.


Bugs: SQOOP-1468
    https://issues.apache.org/jira/browse/SQOOP-1468


Repository: sqoop-SQOOP-1367


Description
-------

The patch is a bit bigger and more intrusive than I've anticipated due to quite huge limitations
that Java imposes on what can be used in annotations. Nevertheless I belive that I've arrived
to quite nice and user friendly solution. The patch is actually not as big as it might seem,
list of changes:

* I've renamed Validator class to AbstractValidator (majority of the changes is just the rename)
* Created new annotation "Validator"
* Instead of taking array of AbstractValidator classes I've changed Class/Form/Input annotations
to accept array of annotations "Validator" - this is important as this is what allows us to
specify arguments and create validators such as "Contains" or "StartsWith"
* Created new validators "Contains" and "StartsWith" and used them in Generic JDBC Connector

Beauty of this solution is that adding a new parameters (like int parameter, ...) is a backward
compatible change provided that the parameter will have default value.


Diffs (updated)
-----

  common/src/main/java/org/apache/sqoop/model/ConfigurationClass.java 5323bd9 
  common/src/main/java/org/apache/sqoop/model/FormClass.java 48bff3c 
  common/src/main/java/org/apache/sqoop/model/Input.java 61fc01a 
  common/src/main/java/org/apache/sqoop/model/Validator.java PRE-CREATION 
  common/src/main/java/org/apache/sqoop/validation/ValidationResult.java 2782fac 
  common/src/main/java/org/apache/sqoop/validation/ValidationRunner.java 46e2d56 
  common/src/main/java/org/apache/sqoop/validation/validators/AbstractValidator.java PRE-CREATION

  common/src/main/java/org/apache/sqoop/validation/validators/ClassAvailable.java 2adfe6c

  common/src/main/java/org/apache/sqoop/validation/validators/Contains.java PRE-CREATION 
  common/src/main/java/org/apache/sqoop/validation/validators/NotEmpty.java 520beea 
  common/src/main/java/org/apache/sqoop/validation/validators/NotNull.java fb8a926 
  common/src/main/java/org/apache/sqoop/validation/validators/StartsWith.java PRE-CREATION

  common/src/main/java/org/apache/sqoop/validation/validators/Validator.java bdb7c20 
  common/src/test/java/org/apache/sqoop/validation/TestValidationRunner.java 1961425 
  common/src/test/java/org/apache/sqoop/validation/validators/TestClassAvailable.java 511b3b4

  common/src/test/java/org/apache/sqoop/validation/validators/TestContains.java PRE-CREATION

  common/src/test/java/org/apache/sqoop/validation/validators/TestNotEmpty.java d225b06 
  common/src/test/java/org/apache/sqoop/validation/validators/TestNotNull.java 9c5bed5 
  common/src/test/java/org/apache/sqoop/validation/validators/TestStartsWith.java PRE-CREATION

  common/src/test/java/org/apache/sqoop/validation/validators/TestValidator.java 1a5dbdd 
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ConnectionForm.java
e513770 
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/FromTableForm.java
1c0b429 
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ToTableForm.java
0601a39 

Diff: https://reviews.apache.org/r/25148/diff/


Testing
-------

Added unit tests for new functionality, existist tests are passing.


Thanks,

Jarek Cecho


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