> On Nov. 14, 2018, 1:53 p.m., Graham Wallis wrote: > > I don't think that this change satisfies the original problem, because even > > with the change Atlas will only perform either a hard or soft delete of an > > entity, subject to configuration of the delete handler. > > > > The requirement in the JIRA is that Atlas should always provide the ability > > to hard delete, regardless of configuration. > > As an optional addition to that, if Atlas is configured for soft delete > > then it will be possible perform a soft delete (optionally followed by > > either a restore or a hard delete). > > > > The purpose of the JIRA is to provide optional soft delete whilst > > continuing to support the mandatory hard delete function. > > > > For the entity store: the way the patch is written, to satisfy the first > > part of the requirement (for provision of hard delete), Atlas would always > > have to be configured for hard delete. It would therefore never be possible > > to soft delete an entity. Technically that meets the requirements, but that > > was the case before the change - provided Atlas was configured for hard > > delete. > > > > I think the change for the relationship store meets the above requirements > > - I think it is possible to call deleteById() and set the forceDelete > > parameter to true. Could we have a similar facility for the entity store? > > Madhan Neethiraj wrote: > Graham - irrespective of the delete-handler specified in the > configuration, a REST API caller can specify the delete-type to use with > query parameter 'deleteType'. For example, to hard-delete an entity the API > call will be like: DELETE > api/atlas/v2/entity/guid/<entity-guid>?deleteType=HARD. Possible values are: > SOFT, HARD and DEFAULT. When this parameter is not specified, the > delete-handler specified in the configuration will be used. > > There is no change to store APIs with this patch. Users of store APIs > should set the desired delete-type by calling > RequestContext.get().setDeleteType(DeleteType) before calling the store APIs. > > Hope this helps. > > Graham Wallis wrote: > OK - thanks Madhan - I am doing this directly from Java so not using the > REST APIs. I will try the RequestContext setter approach. Is there a reason > why the entity store and relationship store appear to handle this very > differently - one having an explicit parameter and the other need the caller > to set the RequestContext first? > > Madhan Neethiraj wrote: > Graham - both entity-store and relationship-store have the same interface > wrt delete-handler. Perhaps you are referring to another patch for this JIRA, > https://reviews.apache.org/r/69305/, where delete-type is taken as parameter > to store APIs. > > Graham Wallis wrote: > I don't agree. It is confusing that there appear to be multiple patches > and reviews relating to this one JIRA, but ignoring the patches and looking > at the latest master, the stores have different interfaces: > > EntityStore: does not offer the caller a force parameter... > > public EntityMutationResponse deleteById(final String guid) throws > AtlasBaseException > > > RelationshipStore: provides the guid-only deleteById method which > delegates to the additional method that accepts the boolean forceDelete... > > public void deleteById(String guid) throws AtlasBaseException { > deleteById(guid, false); } > > public void deleteById(String guid, boolean forceDelete) throws > AtlasBaseException > > > It's not clear what we are recommending to a non-REST caller - should it > set the RequestContext for both stores? Or just for the entity store, and > instead use the explicit forceDelete parameter when deleting a relationship?
Now I see where the confusion is. However, note that RelationshipStore.deleteById(guid, forceDelete) method *was not* introduced in this patch; it has been in place ever since relationship was implemented in 1.0 release. That said, I wish it was named better to avoid such misunderstanding. Even when Atlas is configured for soft-delete, removal of classifications/terms from an entity do delete corresponding edges and classification vertex from the graph. EntityStore has specific method to remove a classification from an entity - deleteClassification(); hence there is no "forceDelete" surfaced in its interface. I wish similar approach was taken in relationship store to deal with removing terms from entities. We should fix this to avoid potential confusion; however, spreading "forceDelete" to more places is not the right direction. For non-REST callers i.e. the code that is integral part of Atlas server itself, calling RequestContext.setDeleteType(deleteType) is the recommended approach. This will cause subsequent operations to be performed as if Atlas was configured for the delete-type (hard/soft) specified in the context - both for entity store and relationship store. - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69334/#review210539 ----------------------------------------------------------- On Nov. 14, 2018, 10:50 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69334/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2018, 10:50 p.m.) > > > Review request for atlas, Ashutosh Mestry, keval bhatt, Kapildeo Nayak, Mehul > Parikh, Nikhil Bonte, Nixon Rodrigues, and Sarath Subramanian. > > > Bugs: ATLAS-2774 > https://issues.apache.org/jira/browse/ATLAS-2774 > > > Repository: atlas > > > Description > ------- > > support "deleteType" query-parameter for REST API calls > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 1b3ce1edf > intg/src/main/java/org/apache/atlas/store/DeleteType.java PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerDelegate.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java > 0bcf0d6cb > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java > 733369692 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java > 21617dcc3 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/BulkImporterImpl.java > a3d31b64d > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java > e8c8c0c8b > repository/src/test/java/org/apache/atlas/TestModules.java d3d30d5d9 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportImportTestBase.java > 4b253ffb7 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportServiceTest.java > 936586ba2 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportSkipLineageTest.java > 3393b82d8 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/SoftReferenceTest.java > a86076464 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java > 4fd28206c > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityTestBase.java > ba0551050 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreHardDeleteV2Test.java > 8955be721 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreSoftDeleteV2Test.java > 82b75daa4 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2Test.java > cd1d72787 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/InverseReferenceUpdateHardDeleteV2Test.java > 5d459081b > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/InverseReferenceUpdateSoftDeleteV2Test.java > 76d6b7d47 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/InverseReferenceUpdateV2Test.java > ea647ecba > server-api/src/main/java/org/apache/atlas/RequestContext.java 9a9bba6a8 > webapp/src/main/java/org/apache/atlas/web/filters/AuditFilter.java > adf3cf21d > > webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityRESTDelete.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/69334/diff/7/ > > > Testing > ------- > > Pre-commit test run - > https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/828 > > > Thanks, > > Madhan Neethiraj > >
