Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/
---

(Updated March 30, 2020, 11:52 p.m.)


Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
Sarath Subramanian.


Bugs: atlas-3700
https://issues.apache.org/jira/browse/atlas-3700


Repository: atlas


Description
---

1) The model definition is in 
"https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;

2) I created a request of type EntityCreateRequestV2. It contains an entity of 
type ml_model_build with an attribute "metadata", which is a map, and contains 
key-value entries for "engineImageTag" and "engineImageName".

3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
an entity of type ml_model_build with an attribute "metadata", which is a map, 
and contains key-value entry for "updated_at".

4) In the properties of the entity ml_model_build, the * attribute "*metadata" 
only contains key-value entry for "updated_at".

Desired behavior:

The attribute "metadata" should contains key-value entries for 
"engineImageTag", "engineImageName" and "updated_at".

Current behavior:

the attribute "metadata" only contains key-value entry for "updated_at".


Diffs (updated)
-

  
intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java 
1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
  intg/src/main/java/org/apache/atlas/model/notification/HookNotification.java 
5b5fa04e26b17071bdc2160974d7005bc110de74 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
 75b016cca04801c4c1512c1527453fd59a11a6c4 
  
repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
 225b72cbef20b2329b36f550da60c1bbe1f95efa 
  server-api/src/main/java/org/apache/atlas/RequestContext.java 
282a66f1d466d58bce7a590513bc92f99733ba81 
  
webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 3f1ea05e17cded3433d050f891c807d55bdd022a 


Diff: https://reviews.apache.org/r/72278/diff/4/

Changes: https://reviews.apache.org/r/72278/diff/3-4/


Testing
---

add new test case for map attribute for partial update


Thanks,

Na Li

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 


> On March 30, 2020, 10:14 p.m., Sarath Subramanian wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
> > Lines 1300 (patched)
> > 
> >
> > The following cases are not handled:
> > 
> > |---|
> > | currValue |  newValue |
> > |---|
> > |  Null |   Null| => Will set to empty map 
> > |  Empty|   Null| => Will set to empty map 
> > |  Not Null |   Null| => Will set to empty map
> > |---|
> > 
> > Please review.
> 
> Karthik Manamcheri wrote:
> Remember to add unit tests for these cases as well.

fixed and added unit test


- Na


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220127
---


On March 30, 2020, 11:52 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 11:52 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> intg/src/main/java/org/apache/atlas/model/notification/HookNotification.java 
> 5b5fa04e26b17071bdc2160974d7005bc110de74 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
>   server-api/src/main/java/org/apache/atlas/RequestContext.java 
> 282a66f1d466d58bce7a590513bc92f99733ba81 
>   
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
>  3f1ea05e17cded3433d050f891c807d55bdd022a 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/4/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Karthik Manamcheri via Review Board 


> On March 30, 2020, 10:14 p.m., Sarath Subramanian wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
> > Lines 1300 (patched)
> > 
> >
> > The following cases are not handled:
> > 
> > |---|
> > | currValue |  newValue |
> > |---|
> > |  Null |   Null| => Will set to empty map 
> > |  Empty|   Null| => Will set to empty map 
> > |  Not Null |   Null| => Will set to empty map
> > |---|
> > 
> > Please review.

Remember to add unit tests for these cases as well.


- Karthik


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220127
---


On March 30, 2020, 9:45 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 9:45 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> intg/src/main/java/org/apache/atlas/model/notification/HookNotification.java 
> 5b5fa04e26b17071bdc2160974d7005bc110de74 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
>   server-api/src/main/java/org/apache/atlas/RequestContext.java 
> 282a66f1d466d58bce7a590513bc92f99733ba81 
>   
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
>  3f1ea05e17cded3433d050f891c807d55bdd022a 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/3/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Sarath Subramanian 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220127
---




repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Lines 1300 (patched)


The following cases are not handled:

|---|
| currValue |  newValue |
|---|
|  Null |   Null| => Will set to empty map 
|  Empty|   Null| => Will set to empty map 
|  Not Null |   Null| => Will set to empty map
|---|

Please review.


- Sarath Subramanian


On March 30, 2020, 2:45 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 2:45 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> intg/src/main/java/org/apache/atlas/model/notification/HookNotification.java 
> 5b5fa04e26b17071bdc2160974d7005bc110de74 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
>   server-api/src/main/java/org/apache/atlas/RequestContext.java 
> 282a66f1d466d58bce7a590513bc92f99733ba81 
>   
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
>  3f1ea05e17cded3433d050f891c807d55bdd022a 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/3/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/
---

(Updated March 30, 2020, 9:45 p.m.)


Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
Sarath Subramanian.


Bugs: atlas-3700
https://issues.apache.org/jira/browse/atlas-3700


Repository: atlas


Description
---

1) The model definition is in 
"https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;

2) I created a request of type EntityCreateRequestV2. It contains an entity of 
type ml_model_build with an attribute "metadata", which is a map, and contains 
key-value entries for "engineImageTag" and "engineImageName".

3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
an entity of type ml_model_build with an attribute "metadata", which is a map, 
and contains key-value entry for "updated_at".

4) In the properties of the entity ml_model_build, the * attribute "*metadata" 
only contains key-value entry for "updated_at".

Desired behavior:

The attribute "metadata" should contains key-value entries for 
"engineImageTag", "engineImageName" and "updated_at".

Current behavior:

the attribute "metadata" only contains key-value entry for "updated_at".


Diffs (updated)
-

  
intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java 
1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
  intg/src/main/java/org/apache/atlas/model/notification/HookNotification.java 
5b5fa04e26b17071bdc2160974d7005bc110de74 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
 75b016cca04801c4c1512c1527453fd59a11a6c4 
  
repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
 225b72cbef20b2329b36f550da60c1bbe1f95efa 
  server-api/src/main/java/org/apache/atlas/RequestContext.java 
282a66f1d466d58bce7a590513bc92f99733ba81 
  
webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 3f1ea05e17cded3433d050f891c807d55bdd022a 


Diff: https://reviews.apache.org/r/72278/diff/3/

Changes: https://reviews.apache.org/r/72278/diff/2-3/


Testing
---

add new test case for map attribute for partial update


Thanks,

Na Li

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 


> On March 30, 2020, 4:46 p.m., Karthik Manamcheri wrote:
> > You mention that the desired behavior is that the Map item should be 
> > appended (instead of overwritten). What do we do for the use-cases where 
> > the desired behavior is to over-write and not append? Does this change 
> > allow the flexibility to specify that?
> 
> Na Li wrote:
> The behavior is controled by the message type: 
> EntityPartialUpdateRequestV2. If the message type is EntityUpdateRequestV2, 
> overwriting will happen.
> 
> Madhan Neethiraj wrote:
> This change will cause existing use of EntityPartialUpdateRequestV2 to 
> break - as this will append to Map type attributes, instead of overwrite. To 
> avoid this issue, I suggest the following:
> - add a flag EntityPartialUpdateRequestV2.isAppendCollectionAttributes. 
> The default value should be false, which would retain the current behavior
> - add a flag in RequestContext.isAppendCollectionAttributes, and set this 
> value in NotificationHookConsumer where ENTITY_PARTIAL_UPDATE_V2 is handled
> - update EntityGraphMapper.mapMapValue() and 
> EntityGraphMapper.mapArrayValue() to perform append/overwrite depending on 
> value of RequestContext.isAppendCollectionAttributes

I have done the following

- add a flag EntityPartialUpdateRequestV2.isAppendCollectionAttributes. The 
default value should be false, which would retain the current behavior
- add a flag in RequestContext.isAppendCollectionAttributes, and set this value 
in NotificationHookConsumer where ENTITY_PARTIAL_UPDATE_V2 is handled
- update EntityGraphMapper.mapMapValue() to perform append/overwrite depending 
on value of RequestContext.isAppendCollectionAttributes

I did not update EntityGraphMapper.mapArrayValue() as it is not clear to me 
what's the reasonable behavior to append. In map, we have key in [key, value] 
to indicate what to append or replace.


- Na


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220113
---


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 


> On March 30, 2020, 4:59 p.m., Karthik Manamcheri wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
> > Lines 1301 (patched)
> > 
> >
> > You can use the .putAll function in HashMap. It does what you want it 
> > to do. It over-writes existing keys and adds non-existing keys.
> > 
> > ```
> >   HashMap currVal = ...
> >   // Add everything from newVal into currVal.
> >   // currVal becomes the merged map.
> >   currVal.putAll(newVal);
> >   ctx.getReferringVertex().setProperty(propertyName, currVal);
> > ```
> > 
> > 
> > https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html#putAll-java.util.Map-
> 
> Na Li wrote:
> currVal could be null
> 
> Karthik Manamcheri wrote:
> Sure then you can just check and create a new map right?
> 
> ```
> // If currVal is null, then there is nothing to merge with. Otherwise, 
> merge the new values
> // with current value.
> HashMap mergedVal = (currVal == null) ? newVal : 
> currVal.putAll(newVal);
> ctx.getReferringVertex().setProperty(propertyName, mergedVal);
> ```

Map newVal
Map mergedMap

Type does not match for putAll()


- Na


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220114
---


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Karthik Manamcheri via Review Board 


> On March 30, 2020, 4:59 p.m., Karthik Manamcheri wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
> > Lines 1301 (patched)
> > 
> >
> > You can use the .putAll function in HashMap. It does what you want it 
> > to do. It over-writes existing keys and adds non-existing keys.
> > 
> > ```
> >   HashMap currVal = ...
> >   // Add everything from newVal into currVal.
> >   // currVal becomes the merged map.
> >   currVal.putAll(newVal);
> >   ctx.getReferringVertex().setProperty(propertyName, currVal);
> > ```
> > 
> > 
> > https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html#putAll-java.util.Map-
> 
> Na Li wrote:
> currVal could be null

Sure then you can just check and create a new map right?

```
// If currVal is null, then there is nothing to merge with. Otherwise, merge 
the new values
// with current value.
HashMap mergedVal = (currVal == null) ? newVal : 
currVal.putAll(newVal);
ctx.getReferringVertex().setProperty(propertyName, mergedVal);
```


- Karthik


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220114
---


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Madhan Neethiraj 


> On March 30, 2020, 4:46 p.m., Karthik Manamcheri wrote:
> > You mention that the desired behavior is that the Map item should be 
> > appended (instead of overwritten). What do we do for the use-cases where 
> > the desired behavior is to over-write and not append? Does this change 
> > allow the flexibility to specify that?
> 
> Na Li wrote:
> The behavior is controled by the message type: 
> EntityPartialUpdateRequestV2. If the message type is EntityUpdateRequestV2, 
> overwriting will happen.

This change will cause existing use of EntityPartialUpdateRequestV2 to break - 
as this will append to Map type attributes, instead of overwrite. To avoid this 
issue, I suggest the following:
- add a flag EntityPartialUpdateRequestV2.isAppendCollectionAttributes. The 
default value should be false, which would retain the current behavior
- add a flag in RequestContext.isAppendCollectionAttributes, and set this value 
in NotificationHookConsumer where ENTITY_PARTIAL_UPDATE_V2 is handled
- update EntityGraphMapper.mapMapValue() and EntityGraphMapper.mapArrayValue() 
to perform append/overwrite depending on value of 
RequestContext.isAppendCollectionAttributes


- Madhan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220113
---


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 


> On March 30, 2020, 4:59 p.m., Karthik Manamcheri wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
> > Lines 1301 (patched)
> > 
> >
> > You can use the .putAll function in HashMap. It does what you want it 
> > to do. It over-writes existing keys and adds non-existing keys.
> > 
> > ```
> >   HashMap currVal = ...
> >   // Add everything from newVal into currVal.
> >   // currVal becomes the merged map.
> >   currVal.putAll(newVal);
> >   ctx.getReferringVertex().setProperty(propertyName, currVal);
> > ```
> > 
> > 
> > https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html#putAll-java.util.Map-

currVal could be null


- Na


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220114
---


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Karthik Manamcheri via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220114
---




repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Line 291 (original), 291 (patched)


You can use ternary operator here to make it a little cleaner.

EntityOperation operation = isPartialUpdate ? PARTIAL_UPDATE : UPDATE



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Lines 1301 (patched)


You can use the .putAll function in HashMap. It does what you want it to 
do. It over-writes existing keys and adds non-existing keys.

```
  HashMap currVal = ...
  // Add everything from newVal into currVal.
  // currVal becomes the merged map.
  currVal.putAll(newVal);
  ctx.getReferringVertex().setProperty(propertyName, currVal);
```


https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html#putAll-java.util.Map-


- Karthik Manamcheri


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 


> On March 30, 2020, 4:46 p.m., Karthik Manamcheri wrote:
> > You mention that the desired behavior is that the Map item should be 
> > appended (instead of overwritten). What do we do for the use-cases where 
> > the desired behavior is to over-write and not append? Does this change 
> > allow the flexibility to specify that?

The behavior is controled by the message type: EntityPartialUpdateRequestV2. If 
the message type is EntityUpdateRequestV2, overwriting will happen.


- Na


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220113
---


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Karthik Manamcheri via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/#review220113
---



You mention that the desired behavior is that the Map item should be appended 
(instead of overwritten). What do we do for the use-cases where the desired 
behavior is to over-write and not append? Does this change allow the 
flexibility to specify that?

- Karthik Manamcheri


On March 30, 2020, 3:58 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72278/
> ---
> 
> (Updated March 30, 2020, 3:58 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
> Sarath Subramanian.
> 
> 
> Bugs: atlas-3700
> https://issues.apache.org/jira/browse/atlas-3700
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> 1) The model definition is in 
> "https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;
> 
> 2) I created a request of type EntityCreateRequestV2. It contains an entity 
> of type ml_model_build with an attribute "metadata", which is a map, and 
> contains key-value entries for "engineImageTag" and "engineImageName".
> 
> 3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
> an entity of type ml_model_build with an attribute "metadata", which is a 
> map, and contains key-value entry for "updated_at".
> 
> 4) In the properties of the entity ml_model_build, the * attribute 
> "*metadata" only contains key-value entry for "updated_at".
> 
> Desired behavior:
> 
> The attribute "metadata" should contains key-value entries for 
> "engineImageTag", "engineImageName" and "updated_at".
> 
> Current behavior:
> 
> the attribute "metadata" only contains key-value entry for "updated_at".
> 
> 
> Diffs
> -
> 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  75b016cca04801c4c1512c1527453fd59a11a6c4 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  225b72cbef20b2329b36f550da60c1bbe1f95efa 
> 
> 
> Diff: https://reviews.apache.org/r/72278/diff/2/
> 
> 
> Testing
> ---
> 
> add new test case for map attribute for partial update
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-30 Thread Na Li via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/
---

(Updated March 30, 2020, 3:58 p.m.)


Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
Sarath Subramanian.


Bugs: atlas-3700
https://issues.apache.org/jira/browse/atlas-3700


Repository: atlas


Description
---

1) The model definition is in 
"https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;

2) I created a request of type EntityCreateRequestV2. It contains an entity of 
type ml_model_build with an attribute "metadata", which is a map, and contains 
key-value entries for "engineImageTag" and "engineImageName".

3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
an entity of type ml_model_build with an attribute "metadata", which is a map, 
and contains key-value entry for "updated_at".

4) In the properties of the entity ml_model_build, the * attribute "*metadata" 
only contains key-value entry for "updated_at".

Desired behavior:

The attribute "metadata" should contains key-value entries for 
"engineImageTag", "engineImageName" and "updated_at".

Current behavior:

the attribute "metadata" only contains key-value entry for "updated_at".


Diffs (updated)
-

  
intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java 
1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
 75b016cca04801c4c1512c1527453fd59a11a6c4 
  
repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
 225b72cbef20b2329b36f550da60c1bbe1f95efa 


Diff: https://reviews.apache.org/r/72278/diff/2/

Changes: https://reviews.apache.org/r/72278/diff/1-2/


Testing
---

add new test case for map attribute for partial update


Thanks,

Na Li

Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite map entry with different keys

2020-03-27 Thread Na Li via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72278/
---

Review request for atlas, Ashutosh Mestry, Karthik Manamcheri, Sridhar K, and 
Sarath Subramanian.


Bugs: atlas-3700
https://issues.apache.org/jira/browse/atlas-3700


Repository: atlas


Description
---

1) The model definition is in 
"https://github.com/apache/atlas/blob/master/addons/models/4000-MachineLearning/4010-ml_model.json#L90;

2) I created a request of type EntityCreateRequestV2. It contains an entity of 
type ml_model_build with an attribute "metadata", which is a map, and contains 
key-value entries for "engineImageTag" and "engineImageName".

3) Then I created a request of type EntityPartialUpdateRequestV2. It contains 
an entity of type ml_model_build with an attribute "metadata", which is a map, 
and contains key-value entry for "updated_at".

4) In the properties of the entity ml_model_build, the * attribute "*metadata" 
only contains key-value entry for "updated_at".

Desired behavior:

The attribute "metadata" should contains key-value entries for 
"engineImageTag", "engineImageName" and "updated_at".

Current behavior:

the attribute "metadata" only contains key-value entry for "updated_at".


Diffs
-

  
intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java 
1434a24590c4f4172378f98d83ca8d9e9ec9c4d8 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
 75b016cca04801c4c1512c1527453fd59a11a6c4 
  
repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
 225b72cbef20b2329b36f550da60c1bbe1f95efa 


Diff: https://reviews.apache.org/r/72278/diff/1/


Testing
---

add new test case for map attribute for partial update


Thanks,

Na Li