MLH-1226 track denorm attributes and add to kafka #5392
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| public Map<String, Map<String, Object>> getAllInternalAttributesMap() { |
There was a problem hiding this comment.
Move the getter later after all intializations
|
|
||
| public Map<String, Map<String, Object>> getAllInternalAttributesMap() { | ||
| return allInternalAttributesMap; | ||
| } |
There was a problem hiding this comment.
This class has a method clearCache(), please clear the allInternalAttributesMap in this method
| if (MapUtils.isNotEmpty(allInternalAttributesMap)) { | ||
| for (String key : allInternalAttributesMap.keySet()) { | ||
| Object value = allInternalAttributesMap.get(key); | ||
| ret.setAttribute(key, value); |
There was a problem hiding this comment.
this will put internal attributes in attributes like attributes.__meanings. Nothing wrong in this just wanted to highlight
| Iterator<? extends Property<String>> it = getWrappedElement().properties(propertyName); | ||
| while(it.hasNext()) { | ||
| Property<String> property = it.next(); | ||
| property.remove(); | ||
| //fill the map for the entityGuid and remove value | ||
| if (propertyName.startsWith(INTERNAL_PROPERTY_KEY_PREFIX)) { |
There was a problem hiding this comment.
This will collect attrs like __createdBy, __createdAt & other attributes which not denorm attrs. shall we exclude them as well?
or we can even have inclusion list but we will loose future readyness & have to update the inclusion list everytime we introduce new denorm attribute
There was a problem hiding this comment.
will do this change after i test this in an environment
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java
Outdated
Show resolved
Hide resolved
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java
Outdated
Show resolved
Hide resolved
| if (propertyName.startsWith(INTERNAL_PROPERTY_KEY_PREFIX)) { | ||
| String entityGuid = this.getProperty(GUID_PROPERTY_KEY, String.class); | ||
| if (StringUtils.isNotEmpty(entityGuid)) { | ||
| if (context.getAllInternalAttributesMap().get(entityGuid) != null) { | ||
| context.getAllInternalAttributesMap().get(entityGuid).put(propertyName, null); | ||
| } else { | ||
| Map<String, Object> map = new HashMap<>(); | ||
| map.put(propertyName, null); | ||
| context.getAllInternalAttributesMap().put(entityGuid, map); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
From method name I think intention is to remove all items from value list
E.g
Imagine __meaningNames current has values ["term_0", "term_1"],
Someone updates asset to remove both term_0, term_1
I will check more on this method & get back
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| @@ -130,25 +130,14 @@ public void removePropertyValue(String propertyName, Object propertyValue) { | |||
| RequestContext context = RequestContext.get(); | |||
| Iterator<? extends Property<Object>> it = getWrappedElement().properties(propertyName); | |||
| // we create a map of properties that are only internal | |||
| fillInternalPropertiesIfApplicable(propertyName, propertyValue, it, context); | |||
| it = getWrappedElement().properties(propertyName); | |||
There was a problem hiding this comment.
I have an idea to avoid iterating twice
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| @@ -109,36 +114,49 @@ public void removeProperty(String propertyName) { | |||
| while(it.hasNext()) { | |||
| Property<String> property = it.next(); | |||
| property.remove(); | |||
| recordInternalAttribute(propertyName, null); | |||
There was a problem hiding this comment.
Bug: Inconsistent Property Removal Logging
The removeProperty method repeatedly calls recordInternalAttribute within its loop for each property value removed. This creates unnecessary overhead and is inconsistent with other remove methods that record the final state once.
Change description
Type of change
Related issues
https://atlanhq.atlassian.net/browse/MLH-1226
Helm Config Changes for Running Tests (Staging PR)
Does this PR require Helm config changes for testing?
enpla9up36. (You can proceed with the PR.) ✅Checklists
Development
Security
Code review