> On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > cli/src/main/java/org/apache/falcon/cli/FalconCLI.java, line 94 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485661#file1485661line94> > > > > Please also add updateClusterDependents, touch, dependencies, list, > > summary, and lookup operations also to the list for the sake of > > completeness.
Done. Added them. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > cli/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java, line 171 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485662#file1485662line171> > > > > description is incorrect, copy-paste error? Had already added EFFECTIVE_TIME_OPT_DESCRIPTION. but missed to put it correctly to this line > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/pom.xml, line 246 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485666#file1485666line246> > > > > Can you please add the version to the parent-pom.xml? Not required. Removing the added dependency > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/pom.xml, line 247 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485666#file1485666line247> > > > > Why is jython required? I couldn't find any usage in the patch. Not required. Removing the added dependency > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 98 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485667#file1485667line98> > > > > If it is specific to process only, then why not move it to > > ProcessHelper.java? Moving them to process helper > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 99 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485667#file1485667line99> > > > > Can you please add comments for all 3 variables explaining what is > > their purpose with example. Added the usage and description of the purpose of each file/directory pattern used. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/ProcessHelper.java, line 96 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485668#file1485668line96> > > > > Please add javadoc for all methods. Done. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/ProcessHelper.java, line 98 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485668#file1485668line98> > > > > try-catch should be put on as tight scope as possible for better > > readability and maintenance. Ack. Modified it > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/ProcessHelper.java, line 100 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485668#file1485668line100> > > > > Please use StringUtils.isBlank() Ack. Changed it. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/ProcessHelper.java, line 252 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485668#file1485668line252> > > > > Same as above for try-catch scope. Ack. Modified it. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 58 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line58> > > > > Please add javadocs for all public methods. Ack. Added > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 50 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line50> > > > > Please explain in the comments what is Lib Dictionary. Ack. Added comments. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 68 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line68> > > > > Please use try-with resources. Try with resources cannot be used, as checkstyle issues around try-catch resources in java. The code in check style has been fixed recently, but can be used after the latest release of checkstyles plugin. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 72 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line72> > > > > Please add logging with appropriate context. Added logging. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 81 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line81> > > > > libCheckSumsMap will be a better name. Ack. Modified > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 89 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line89> > > > > Same as above for try-with resources. Same as above. Try with resources cannot be used, as checkstyle issues around try-catch resources in java. The code in check style has been fixed recently, but can be used after the latest release of checkstyles plugin. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 94 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line94> > > > > Please add logging. Added logging > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 98 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line98> > > > > Rename to createLibCheckSumsDictionary Modified it to createAndWriteLibDictionary > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 99 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line99> > > > > Input parameter should be Process instead of entity as it is only > > called for process and not other entities. Changed it. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 110 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line110> > > > > If the path doesn't exist then fs.exists will return false. How is it > > sure in case of IOException that the file doesn't exist? Falling back to creating new copy in case of IOException. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 113 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line113> > > > > Staging path in context of Falcon has a special meaning, it will be > > better to reword the message and rename the variable to convey that it is > > first moved to a temporary location. Modified the comment. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 115 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line115> > > > > Unused variable md5 EntityUtil.getNewStagingPath is being called in next line which uses md5. This variable is therefore not required. Removing it. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 119 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line119> > > > > won't workflowLibPath convey the meaning better? No. workflow can be falcon workflow too. This specifically conveys the user defined path. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 122 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line122> > > > > It is never called for feed, so please remove the type check and the > > else if part. Yup. Modified to remove the feed part. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 126 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line126> > > > > Again inconsistent naming convention, > > the separator is called WF_LIB_SEPARATOR and path is called > > userDefinedLibPath. Please make everything consistent to follow workflow > > lib semantics modified it to USER_LIB_SEPARATOR > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 131 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line131> > > > > If wf_lib path contains multiple directories with same jar, how does > > oozie handle it? Is our behavior consistent with that? Oozie passes it as the list of the jars as it is. To have only the one of the jar in the lib path is more a functionality to let user application get only 1 jar rather than duplicating when its the same jar. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 135 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line135> > > > > The if statement can be simplified, by starting with an empty hash map > > instead of null. > > > > Here the empty check is redundant. Made the code better now. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 144 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line144> > > > > Can you please document it in detail in the user docs, JIRA and the > > design doc > > 1. how the jars are getting stored, which path, permissions of the > > directories and the jars etc. > > 2. how are the duplicates handled 1) I will update the design Doc and the user docs for the same. 2) Duplicates won't be handled, only one of them would go. This is not a gap, which has to be fixed. This has to be conveyed to the user in proper addressing. I will add this as part of final documentation. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 148 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line148> > > > > Please log the error message. Logged it. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 155 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line155> > > > > Please avoid using stagingPath in variable names as it has a special > > meaning in context of Falcon. modified it. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 159 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line159> > > > > This check can be done upfront and we can avoid creating staging dir if > > jarPath doesn't exist. Modified Accordingly. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 168 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line168> > > > > Please add javadoc. Added > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java, > > line 195 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485669#file1485669line195> > > > > Name of the method says earliest, but it seems here that the larger > > modification time implies file is smaller and hence will appear first in > > the sorted list. Thanks for catching it. Corrected the condition. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityLibDictionary.java, > > line 24 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485670#file1485670line24> > > > > Name is a misnomer as it is not a dictionary(set of key-value pairs), > > please change. Again, inconsistent naming between EntityLibDictionary and > > ProcessLibDictionary across the codebase. This can be later on be used by Feed too, so keeping it in the convention EntityLib Modifiying the class name to EntityLibEntry > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityLibDictionary.java, > > line 30 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485670#file1485670line30> > > > > Rename parameter to libPathWithChecksum. Changed it. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/entity/v0/EntityLibDictionary.java, > > line 45 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485670#file1485670line45> > > > > setters are not being used anywhere. Removing them. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java, > > line 140 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485672#file1485672line140> > > > > This doesn't compile! Code had changed on the pull and rebase. The method parameters have changed. Parent Id is no more required. Removing the third parameter. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > src/build/checkstyle.xml, line 122 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485695#file1485695line122> > > > > Let's keep it at default value of 150. It is good to ensure readability > > and maintenance. Ack. changed it to 150 > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, > > line 582 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485693#file1485693line582> > > > > Should be UnsupportedOperationException. Removed this method as it doesn't have to be a public method and in which case not required in FalconWorkflowEngine. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java, > > line 577 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485693#file1485693line577> > > > > Why this change is required? Removed this method as it doesn't have to be a public method and in which case not required in FalconWorkflowEngine. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java, > > line 313 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485692#file1485692line313> > > > > Looks like the code allows for effective time to be passed for all > > types of entities (including clusters as per recent cluster update feature). Added code to restrict the effective time param to be checked along with entity type and throw exception if not. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, > > line 1481 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485690#file1485690line1481> > > > > The if part of the if-else is not required. Ack.Corrected it > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, > > line 359 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485690#file1485690line359> > > > > It will be nice to check it at the start of the update API method. This > > will ensure that check is done for all code branches without repeating it > > again and again. This check needs to be done for each cluster under a process. Upate api anways has to be called for each cluster. So best to call it from here. > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > oozie/src/main/java/org/apache/falcon/oozie/OozieBundleBuilder.java, line > > 118 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485676#file1485676line118> > > > > Intuitively, it doesn't make sense to move this code to its parent > > class OozieEntityBuilder as we are changing the libpath only for process. Handled it accordingly to let his API being called only for process builders > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > oozie/src/main/java/org/apache/falcon/oozie/OozieEntityBuilder.java, line > > 302 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485678#file1485678line302> > > > > Better will be to make a new function called getProcessLibPath and > > leave methods for other entities unchanged. Handled it accordingly to let his API being called only for process builders > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedBundleBuilder.java, > > line 85 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485680#file1485680line85> > > > > getLibPath doesn't require cluster for anything except process, so it > > will make the code much more readable and maintainable if we could keep > > this existing definition as it is(may need to remove @override). Handled it accordingly to let his API being called only for process builders > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedReplicationWorkflowBuilder.java, > > line 82 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485681#file1485681line82> > > > > Since there is no change for feeds, these files shouldn't be changed. > > If a different method signature is required for process, then only in > > builders for process change should be reflected, probably by overloading / > > providing a new specific method for process, along with a comment on why > > this special handling is required. Handled it accordingly to let his API being called only for process builders > On Aug. 28, 2016, 6:25 p.m., Praveen Adlakha wrote: > > oozie/src/main/java/org/apache/falcon/oozie/process/ProcessBundleBuilder.java, > > line 114 > > <https://reviews.apache.org/r/51424/diff/1/?file=1485686#file1485686line114> > > > > remove dead code. Removed dead code. - sandeep ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51424/#review147081 ----------------------------------------------------------- On Oct. 19, 2016, 3:03 p.m., sandeep samudrala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51424/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2016, 3:03 p.m.) > > > Review request for Falcon and Pallavi Rao. > > > Repository: falcon-git > > > Description > ------- > > Effective Time in Entity Update > > > Diffs > ----- > > cli/src/main/java/org/apache/falcon/cli/FalconCLI.java 0dd11f6 > cli/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java a8aea52 > client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java > 5d6eff5 > client/src/main/java/org/apache/falcon/client/FalconCLIConstants.java > 04f1599 > client/src/main/java/org/apache/falcon/client/FalconClient.java 8f77fad > common/src/main/java/org/apache/falcon/entity/ClusterHelper.java f89def3 > common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java > PRE-CREATION > common/src/main/java/org/apache/falcon/entity/EntityLibEntry.java > PRE-CREATION > common/src/main/java/org/apache/falcon/entity/EntityUtil.java 8fe316c > common/src/main/java/org/apache/falcon/entity/ProcessHelper.java e563d18 > > common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java > 38fa3ae > common/src/main/java/org/apache/falcon/update/UpdateHelper.java 266319f > > common/src/main/java/org/apache/falcon/workflow/engine/AbstractWorkflowEngine.java > 16a1753 > common/src/test/java/org/apache/falcon/entity/EntityDictionaryUtilTest.java > PRE-CREATION > common/src/test/java/org/apache/falcon/update/UpdateHelperTest.java 826686f > docs/src/site/twiki/falconcli/Touch.twiki afbd848 > docs/src/site/twiki/falconcli/UpdateEntity.twiki 146a60f > oozie/src/main/java/org/apache/falcon/oozie/OozieBundleBuilder.java 5f93cc2 > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedBundleBuilder.java > c758411 > > oozie/src/main/java/org/apache/falcon/oozie/process/HiveProcessWorkflowBuilder.java > 9f9579c > > oozie/src/main/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilder.java > f93a599 > > oozie/src/main/java/org/apache/falcon/oozie/process/PigProcessWorkflowBuilder.java > a1a7c12 > > oozie/src/main/java/org/apache/falcon/oozie/process/ProcessBundleBuilder.java > 6661dd5 > > oozie/src/main/java/org/apache/falcon/oozie/process/ProcessExecutionCoordinatorBuilder.java > 91f4757 > > oozie/src/main/java/org/apache/falcon/oozie/process/ProcessExecutionWorkflowBuilder.java > 20eeffd > > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java > 394600c > > oozie/src/test/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilderTest.java > 05b513e > oozie/src/test/resources/config/process/dumb-hive-process.xml c504074 > oozie/src/test/resources/config/process/hive-process-FSInputFeed.xml > d871377 > oozie/src/test/resources/config/process/hive-process-FSOutputFeed.xml > 23d96c3 > oozie/src/test/resources/config/process/hive-process.xml 4dac8e9 > oozie/src/test/resources/config/process/pig-process-0.1.xml 8d20cee > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > aefd699 > > prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java > 3bdeb99 > > prism/src/main/java/org/apache/falcon/resource/extensions/ExtensionManager.java > 92b5531 > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java > 07334d6 > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java > 9ba62a1 > > shell/src/main/java/org/apache/falcon/shell/commands/FalconEntityCommands.java > 35a6f2a > src/build/checkstyle.xml 292a0a3 > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java 53073f0 > > unit/src/main/java/org/apache/falcon/unit/LocalSchedulableEntityManager.java > 7398c8a > unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java bfc8b08 > unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 0bc7755 > unit/src/test/resources/process.xml 6854311 > webapp/src/main/java/org/apache/falcon/resource/ConfigSyncService.java > 7b32bd5 > > webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java > 5525207 > webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java > 876ada5 > > Diff: https://reviews.apache.org/r/51424/diff/ > > > Testing > ------- > > > Thanks, > > sandeep samudrala > >
