On Sept. 21, 2016, 7:46 a.m., Sarath Kumar Subramanian wrote: > > Add tests > > > > 1. Currently, the model files(like hive_model.json) are auto generated from > > model definitions defined in java(like HiveDataModelGenerator). The patch > > files in this case has to be hand coded which is error prone > > 2. For completeness, readability and debuggability, the type update has to > > be done in the corresponding model definitions like HiveDataModelGenerator. > > So, same data will be in two places and the model definitions and the patch > > files can go out of sync > > 3. Since the model definitions(like HiveDataModelGenerator) will be updated > > anyways, if we modify ReservedTypesRegistrar to do type update instead of > > type create, the type updates will automatically be taken care with the > > same model json. So, model update patches are not necessary then > > 3. This jira doesn't implement type versioning - doesn't have support for > > storing multiple versions of the type. But it maintains the version of the > > latest type definition which I think is useful for debugging, to know the > > version of type that the server knows. Can we maintain this info in > > HiveDataModelGenerator itself, and hence will be part of hive_model.json > > Sarath Kumar Subramanian wrote: > 1. patch json genarator can be addressed in a separate jiira > 2. this is only for upgrade scenarios > 3. lets address this in a separate jiira. > 4. we dont want to maintain multiple versions of the same type in the > typesystem, will confuse users on which version to use to create entities. > > Shwetha GS wrote: > What benefit does this patch framework add when the type update can be > done with the existing functionality itself (Type versioning is useful and > can be done without the patch framework). The patch framework adds more > overhead, and I don't see the necessity > > David Radley wrote: > I agree - I think this capability would be better placed in separate > tooling that uses the Atlas REST APIs. > > Madhan Neethiraj wrote: > @Shwetha, @David - the rational for the patch framework is exactly same > as the need for reading contents of "models" directory during startup and > initializing the typesystem. This just makes it easier to deal with > updates/additions to typesystem during software upgrade/patch. > > Sarath Kumar Subramanian wrote: > The existing system to update types (using REST) has these limitations: > > 1. Clients might add new attributes to a type and when we add a set of > new attributes in model.json - the rest api model overwrites customer > changes. > > 2. More granular type updates cannot be applied, if you add a new > attribute in hive_model for eg., During atlas startup it doesn't do a type > by model comparison and apply the diff. This patch can update to a specific > type based on its version - condition based update. > > David Radley wrote: > @Madhan @Sarath It seems to me that the model files - are system types > that we supply to aid integration with Hive and the like. As these do not > change, then I am happy these are in model files. If we put changes in patch > files, then patch files become the master. I think we need the Atlas > repository to be the master of all the types in all cases. It should not be > a slave to patch files. I can see a place for patch files that tooling uses > at a one off upgrade of types to update the repository. I agree that care > needs to be taken on type shape changes, in practice I would expect that > these shape change operations could be restricted to admin roles. > > Shwetha GS wrote: > Sarath, > >>Clients might add new attributes to a type and when we add a set of new > attributes in model.json - the rest api model overwrites customer changes > Thats a valid point. > > The patch doesn't apply on trunk. Can you re-base the patch and add the > tests? > > David, > Registering these models/patches is part of set-up tool required for > hooks to work. These use internal methods directly which we should change to > make use of APIs. This set-up is required to be run before any hooks can > register the entities, hence its run as part of service start itself. But I > agree that we should move this out of repository.
Shwetha, I have rebased the patch and added unit tests. Thanks, Sarath - Sarath Kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51939/#review149811 ----------------------------------------------------------- On Sept. 26, 2016, 3:27 p.m., Sarath Kumar Subramanian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51939/ > ----------------------------------------------------------- > > (Updated Sept. 26, 2016, 3:27 p.m.) > > > Review request for atlas, Madhan Neethiraj, Shwetha GS, and Suma Shivaprasad. > > > Bugs: ATLAS-1174 > https://issues.apache.org/jira/browse/ATLAS-1174 > > > Repository: atlas > > > Description > ------- > > 1. Introduce "version" attribute to all types in the type-system, this helps > to track changes made to the default types (hive, sqoop, falcon and storm > types) and user created types. If version is not mentioned during creation of > a type, default version "1.0" is assigned (optional attribute). > 2. Using the version attributed for types, introduce a patch framework for > type system. This framework applies patches to a type using its version > number and can be used during upgrade - add new attributes to an existing > types and it will be run during atlas startup. > The sequence of steps: > a. During atlas startup, check $ATLAS_HOME/models/patches directory for any > available patch files (json files). If there any patch files handle them. > b. Sample patch json file looks like: > { > "patches": [ > { > "action": "ADD_ATTRIBUTE", > "typeName": "hive_column", > "applyToVersion": "1.0", > "updateToVersion": "2.0", > "actionParams": [ > { "name": "position", "dataTypeName": "int", "multiplicity": "optional", > "isComposite": false, "isUnique": false, "isIndexable": false, > "reverseAttributeName": null } > ] > } ] > } > c. The framework updates the type in "typeName" for the matching version > number and after applying the patch, update the version to the one mentioned > in "updateToVersion" > d. The json file can have more than one action (array of actions). > e. There can be multiple patch json files in the directory and are applied in > the sort order of the filename. eg: > 001-hive_column_add_position.json > 002-hive_column_add_anotherattribute.json > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/AtlasConstants.java 17ffbd7 > common/src/main/java/org/apache/atlas/repository/Constants.java d7f9c89 > > repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java > a94d157 > repository/src/main/java/org/apache/atlas/services/AtlasPatchHandler.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/services/AtlasTypeAttributePatch.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/services/AtlasTypePatch.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java > 3550492 > > repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java > 6782970 > > typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java > fad091d > typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java > c56987a > typesystem/src/main/java/org/apache/atlas/typesystem/types/EnumType.java > bdd0a13 > > typesystem/src/main/java/org/apache/atlas/typesystem/types/EnumTypeDefinition.java > 6340615 > > typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java > 7224699 > > typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalTypeDefinition.java > 9a299f0 > typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java > 85ddee7 > typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java > 6f40c1d > > typesystem/src/main/java/org/apache/atlas/typesystem/types/StructTypeDefinition.java > f1ce1b7 > typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java > f23bf5b > typesystem/src/main/java/org/apache/atlas/typesystem/types/TypeSystem.java > 70ba89b > > typesystem/src/main/java/org/apache/atlas/typesystem/types/utils/TypesUtil.java > ef8448d > > typesystem/src/main/scala/org/apache/atlas/typesystem/json/TypesSerialization.scala > 5618938 > > Diff: https://reviews.apache.org/r/51939/diff/ > > > Testing > ------- > > > Thanks, > > Sarath Kumar Subramanian > >
