falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ajay Yadava" <ajayn...@gmail.com>
Subject Re: Review Request 39588: State Store for instances scheduled by Falcon Native Scheduler
Date Thu, 05 Nov 2015 10:50:53 GMT

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



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
143)
<https://reviews.apache.org/r/39588/#comment163708>

    what are we really testing here? You have already asserted that row for entity id doesn't
exist, then why call update or delete or get on it?



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
170)
<https://reviews.apache.org/r/39588/#comment163710>

    If you really want to assert the number of rows in result then may be it is a good idea
to put an assert of 0 results before, at this place.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
176)
<https://reviews.apache.org/r/39588/#comment163709>

    You should assert the collection with expected collection i.e. assert the names as well.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
211)
<https://reviews.apache.org/r/39588/#comment163711>

    same as above.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
219)
<https://reviews.apache.org/r/39588/#comment163712>

    same as above.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
226)
<https://reviews.apache.org/r/39588/#comment163713>

    same as above.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
233)
<https://reviews.apache.org/r/39588/#comment163714>

    same as above.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
243)
<https://reviews.apache.org/r/39588/#comment163715>

    Please break it into several small tests instead of one large test.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
325)
<https://reviews.apache.org/r/39588/#comment163716>

    please create separate tests for each type of operation like delete, update etc.



scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java (line
340)
<https://reviews.apache.org/r/39588/#comment163717>

    Use equals instead of writing such methods.


- Ajay Yadava


On Nov. 3, 2015, 5:45 p.m., pavan kumar kolamuri wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39588/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2015, 5:45 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/FALCON-1234
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/FALCON-1234
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Persistent State Store for Falcon Native Scheduler
> 
> 
> Diffs
> -----
> 
>   checkstyle/src/main/resources/falcon/checkstyle.xml 2130e73 
>   checkstyle/src/main/resources/falcon/findbugs-exclude.xml 0a7580d 
>   common/src/main/resources/startup.properties cc5212a 
>   pom.xml 6f2c480 
>   scheduler/pom.xml 20a91d2 
>   scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java 3869ff2

>   scheduler/src/main/java/org/apache/falcon/execution/FalconExecutionService.java b959320

>   scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java 19089c4

>   scheduler/src/main/java/org/apache/falcon/predicate/Predicate.java fb4ce82 
>   scheduler/src/main/java/org/apache/falcon/state/ID.java 420c856 
>   scheduler/src/main/java/org/apache/falcon/state/StateService.java 81357a4 
>   scheduler/src/main/java/org/apache/falcon/state/store/EntityStateStore.java 4aa6fdb

>   scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java 3822860

>   scheduler/src/main/java/org/apache/falcon/state/store/InstanceStateStore.java d6a4b49

>   scheduler/src/main/java/org/apache/falcon/state/store/StateStore.java f595c26 
>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java PRE-CREATION

>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/EntityBean.java PRE-CREATION

>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java PRE-CREATION

>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java PRE-CREATION

>   scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java
PRE-CREATION 
>   scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java PRE-CREATION

>   scheduler/src/main/resources/META-INF/persistence.xml PRE-CREATION 
>   scheduler/src/main/resources/falcon-buildinfo.properties PRE-CREATION 
>   scheduler/src/test/java/org/apache/falcon/execution/FalconExecutionServiceTest.java
b2f9e59 
>   scheduler/src/test/java/org/apache/falcon/notification/service/SchedulerServiceTest.java
b4a0f35 
>   scheduler/src/test/java/org/apache/falcon/state/AbstractSchedulerTestBase.java PRE-CREATION

>   scheduler/src/test/java/org/apache/falcon/state/EntityStateServiceTest.java 2f32b43

>   scheduler/src/test/java/org/apache/falcon/state/InstanceStateServiceTest.java d27ac7e

>   scheduler/src/test/java/org/apache/falcon/state/service/TestFalconJPAService.java PRE-CREATION

>   scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java
PRE-CREATION 
>   scheduler/src/test/java/org/apache/falcon/tools/TestFalconStateStoreDBCLI.java PRE-CREATION

>   scheduler/src/test/resources/startup.properties PRE-CREATION 
>   src/bin/falcon-db.sh PRE-CREATION 
>   src/conf/startup.properties ce6e91f 
>   src/main/assemblies/distributed-package.xml 794eaef 
>   src/main/assemblies/standalone-package.xml fcff8d7 
>   unit/src/main/resources/startup.properties fe6f430 
> 
> Diff: https://reviews.apache.org/r/39588/diff/
> 
> 
> Testing
> -------
> 
> I have written unit tests. I will also test externally by setting up everything
> 
> 
> Thanks,
> 
> pavan kumar kolamuri
> 
>


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