> On Oct. 2, 2019, 7:56 p.m., Karthik Manamcheri wrote: > > addons/ml-bridge-shim/src/main/java/org/apache/atlas/ml/governance/hooks/GovernanceLineageHook.java > > Lines 28 (patched) > > <https://reviews.apache.org/r/71568/diff/1/?file=2167615#file2167615line28> > > > > This comment is confusing. What is the ML Governance server? I think > > that's an internal CDSW detail. > > Na Li wrote: > Does this comment works for you "This class is used to convert machine > learning metadata to lineage notifications > and send them to Atlas."? > > If not, what's your suggestion? > > Karthik Manamcheri wrote: > That works. Mention CDSW as well since this understands only CDSW-style > metadata. There is no universal machine learning metadata. > > "This class is used to convert machine learning metadata from CDSW into > lineage notifications"
The feedback from Atlas team is to not mention CDSW. > On Oct. 2, 2019, 7:56 p.m., Karthik Manamcheri wrote: > > addons/ml-bridge/src/main/java/org/apache/atlas/ml/governance/model/MlLifecycleInfo.java > > Lines 33 (patched) > > <https://reviews.apache.org/r/71568/diff/1/?file=2167624#file2167624line33> > > > > A general comment applicable to your naming convention. Any object > > which represents a CDSW object should be prefixed with CDSW_something. > > > > For example, this representation of "lifecycle" is very CDSW specific > > and we should call it that. For exmaple, CdswLifecycleInfo > > > > This comment applies to all the objects which you are parsing out of > > CDSW Json. > > Na Li wrote: > the fields are in json string. Info from other sources can be formated to > a json string with those fields. That is why I named the classes with more > general names, not CDSW specific. Based on feedback from Atlas team, we should not mention CDSW > On Oct. 2, 2019, 7:56 p.m., Karthik Manamcheri wrote: > > addons/models/1000-Hadoop/1110-ml_model.json > > Lines 11 (patched) > > <https://reviews.apache.org/r/71568/diff/1/?file=2167654#file2167654line11> > > > > What is "serviceType" used for? In the future, if I want to extend this > > system for non-cdsw use-cases should I change this? “serviceType” is a grouping of related entity typedefs e.g.“hive”, “impala”. I changed the service type to "ml". for other use-cases, we can use a different name. > On Oct. 2, 2019, 7:56 p.m., Karthik Manamcheri wrote: > > addons/models/1000-Hadoop/1110-ml_model.json > > Lines 204 (patched) > > <https://reviews.apache.org/r/71568/diff/1/?file=2167654#file2167654line204> > > > > I don't think we should have training data represented here. We don't > > have a way to pull training data from CDSW. Let's not overdefine without > > implementation. I removed the way to get training data. > On Oct. 2, 2019, 7:56 p.m., Karthik Manamcheri wrote: > > pom.xml > > Lines 1574 (patched) > > <https://reviews.apache.org/r/71568/diff/1/?file=2167655#file2167655line1574> > > > > Same goes here. This should be cdsw-bridge. the hook API is general. should not be CDSW specific. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71568/#review218027 ----------------------------------------------------------- On Oct. 2, 2019, 11:31 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71568/ > ----------------------------------------------------------- > > (Updated Oct. 2, 2019, 11:31 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/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/cdswModelDeployments.json PRE-CREATION > addons/ml-bridge/src/test/resources/cdswModelDeploymentsTrainingData.json > PRE-CREATION > addons/ml-bridge/src/test/resources/cdswModels.json PRE-CREATION > addons/ml-bridge/src/test/resources/cdswModelsTrainingData.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/2/ > > > Testing > ------- > > add integration tests > > > Thanks, > > Na Li > >
