vyommani commented on PR #1112: URL: https://github.com/apache/ranger/pull/1112#issuecomment-5145492880
> @vyommani - how about adding `normalize()` method in `RangerServiceDef` which will perform the operation in thread-safe way. This will isolate the details within `RangerServiceDef`. Good suggestion, and it's true that today's ServiceDefUtil.normalizeDataMaskDef()/normalizeRowFilterDef() are technically callable directly on an un-copied instance, which is a real gap worth closing eventually. One clarification on the "thread-safe" framing: the current fix doesn't need any synchronization at all - normalize() returns a fresh copy with no shared mutable state, so there's nothing left to lock. Moving it onto RangerServiceDef wouldn't change that; it'd still be copy-then-mutate, just called as serviceDef.normalize() instead of ServiceDefUtil.normalize(serviceDef). We'd rather not fold this into the current PR. Making it worthwhile (not just a thin wrapper) means normalizeDataMaskDef()/normalizeRowFilterDef() becoming private instance methods only reachable through the new normalize(), which means re-touching every call site we just finished auditing and testing , plus a decision on whether normalizeAccessTypeDefs()/getMarkerAccessTypes()/getDataMaskType() move too (leaving them as static utilities would create a half-migrated, inconsistent API). None of that is hard, but it's real scope on a patch that just went green. We'll file this as a follow-up and take a pass at moving the whole normalize-family onto the model class in one coherent change, rather than partially migrating it here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
