----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71568/#review218119 -----------------------------------------------------------
addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/MlGovernanceHookTool.java Lines 145-169 (patched) <https://reviews.apache.org/r/71568/#comment305682> I would recommend at least switching to wget instead of using curl if we are going to kick the can on switching this to an HTTP client. Curl is not installed by default and could/will cause issues. I know this was an issue with Impala relying on curl. addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java Lines 67 (patched) <https://reviews.apache.org/r/71568/#comment305683> I think the comment needs to be removed if this is reolved or an issue opened for the default. addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java Lines 110 (patched) <https://reviews.apache.org/r/71568/#comment305684> Does some clean up need to occur in the 'finally' block? If not can we remove it? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java Lines 123-127 (patched) <https://reviews.apache.org/r/71568/#comment305685> It appears nothing is happening here. Can we remove this code? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java Lines 137 (patched) <https://reviews.apache.org/r/71568/#comment305686> Do we want an OR here instead of an AND? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java Lines 181 (patched) <https://reviews.apache.org/r/71568/#comment305687> hit: s/unknown/not supported/ addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/events/BaseMlGovernanceEvent.java Lines 80 (patched) <https://reviews.apache.org/r/71568/#comment305673> nit: because the variable name is too long it is not aligned with everything else. addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/events/BaseMlGovernanceEvent.java Lines 203 (patched) <https://reviews.apache.org/r/71568/#comment305693> nit: what is this spacing? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/events/BaseMlGovernanceEvent.java Lines 221-222 (patched) <https://reviews.apache.org/r/71568/#comment305692> nit: what is this spacing? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/events/BaseMlGovernanceEvent.java Lines 249 (patched) <https://reviews.apache.org/r/71568/#comment305691> nit: what is this spacing? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/events/BaseMlGovernanceEvent.java Lines 272 (patched) <https://reviews.apache.org/r/71568/#comment305690> nit: what is this spacing? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModel.java Lines 54-66 (patched) <https://reviews.apache.org/r/71568/#comment305675> nit: should these methods be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModel.java Lines 73-79 (patched) <https://reviews.apache.org/r/71568/#comment305674> nit: Should these 2 methods also be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelDeployment.java Lines 59-61 (patched) <https://reviews.apache.org/r/71568/#comment305676> nit: should this be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelDeployment.java Lines 67-69 (patched) <https://reviews.apache.org/r/71568/#comment305677> nit: should this be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelDeployment.java Lines 73-75 (patched) <https://reviews.apache.org/r/71568/#comment305678> nit: should this be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelDeployment.java Lines 85-87 (patched) <https://reviews.apache.org/r/71568/#comment305679> nit: should this be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelDeployment.java Lines 89-91 (patched) <https://reviews.apache.org/r/71568/#comment305680> nit: should this be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelDeployment.java Lines 99-101 (patched) <https://reviews.apache.org/r/71568/#comment305681> nit: should this be one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlReplicaStatus.java Lines 43-45 (patched) <https://reviews.apache.org/r/71568/#comment305694> nit: Should this be on one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlReplicationPolicy.java Lines 35 (patched) <https://reviews.apache.org/r/71568/#comment305696> nit: spacing after 'int' is not consistent with other classes of this type. addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlReplicationPolicy.java Lines 40-42 (patched) <https://reviews.apache.org/r/71568/#comment305695> nit: Should this be on one line? addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlUser.java Lines 34 (patched) <https://reviews.apache.org/r/71568/#comment305697> nit: spacing after 'int' is not consistent with other similar classes - Austin Nobis On Oct. 4, 2019, 8:21 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71568/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2019, 8:21 p.m.) > > > Review request for atlas, Austin Nobis, Ashutosh Mestry, and Sarath > Subramanian. > > > Repository: atlas > > > Description > ------- > > add integration for ML > > > Diffs > ----- > > addons/hive-bridge/pom.xml 13384cb > addons/ml-bridge-shim/pom.xml PRE-CREATION > > addons/ml-bridge-shim/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java > PRE-CREATION > addons/ml-bridge/README.md PRE-CREATION > addons/ml-bridge/pom.xml PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/HookToolConfiguration.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/MlGovernanceHookTool.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/AtlasMlGovernanceProcessContext.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/events/BaseMlGovernanceEvent.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/hooks/events/CreateMlGovernanceLineage.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlDataBase.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlDataType.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlLifecycleInfo.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModel.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelBuild.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelDeployment.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlModelRef.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlProject.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlReplicaStatus.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlReplicationPolicy.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlReplicationStatus.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlResources.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlTrainingData.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlTrainingDataEntry.java > PRE-CREATION > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlUser.java > PRE-CREATION > addons/ml-bridge/src/main/resources/atlas-log4j.xml PRE-CREATION > addons/ml-bridge/src/main/resources/governance_hook.sh PRE-CREATION > > addons/ml-bridge/src/test/java/org/apache/atlas/ml/governance/GovernanceHookIT.java > PRE-CREATION > > addons/ml-bridge/src/test/java/org/apache/atlas/ml/governance/GovernanceHookITBase.java > PRE-CREATION > addons/ml-bridge/src/test/resources/atlas-application.properties > PRE-CREATION > addons/ml-bridge/src/test/resources/atlas-log4j.xml PRE-CREATION > addons/ml-bridge/src/test/resources/mlModelDeployments.json PRE-CREATION > addons/ml-bridge/src/test/resources/mlModelDeploymentsTrainingData.json > PRE-CREATION > addons/ml-bridge/src/test/resources/mlModels.json PRE-CREATION > addons/ml-bridge/src/test/resources/mlModelsTrainingData.json PRE-CREATION > addons/ml-bridge/src/test/resources/users-credentials.properties > PRE-CREATION > addons/ml-hook-api/pom.xml PRE-CREATION > > addons/ml-hook-api/src/main/java/org/apache/mlops/governance/hooks/Entity.java > PRE-CREATION > > addons/ml-hook-api/src/main/java/org/apache/mlops/governance/hooks/EntityType.java > PRE-CREATION > > addons/ml-hook-api/src/main/java/org/apache/mlops/governance/hooks/GovernanceHook.java > PRE-CREATION > > addons/ml-hook-api/src/main/java/org/apache/mlops/governance/hooks/HookContext.java > PRE-CREATION > > addons/ml-hook-api/src/main/java/org/apache/mlops/governance/hooks/MethodType.java > PRE-CREATION > addons/models/1000-Hadoop/1110-ml_model.json PRE-CREATION > pom.xml ce7d6fb > > > Diff: https://reviews.apache.org/r/71568/diff/3/ > > > Testing > ------- > > add integration tests > > > Thanks, > > Na Li > >
