You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Na Li via Review Board <no...@reviews.apache.org> on 2020/03/27 22:39:08 UTC
Review Request 72278: ATLAS-3700: Partial update to Atlas overwrite
map entry with different keys
-----------------------------------------------------------
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
Re: Review Request 72278: ATLAS-3700: Partial update to Atlas
overwrite map entry with different keys
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/72278/diff/2/?file=2216459#file2216459line1307>
> >
> > 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<String, Object> 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<String, Object> mergedVal = (currVal == null) ? newVal : currVal.putAll(newVal);
> ctx.getReferringVertex().setProperty(propertyName, mergedVal);
> ```
Map<Object, Object> newVal
Map<String, Object> 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
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/72278/diff/2/?file=2216459#file2216459line1307>
> >
> > 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<String, Object> 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<String, Object> 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
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/72278/diff/2/?file=2216459#file2216459line1307>
> >
> > 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<String, Object> 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
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/72278/#comment308442>
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)
<https://reviews.apache.org/r/72278/#comment308443>
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<String, Object> 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
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/72278/diff/2/?file=2216459#file2216459line1306>
> >
> > 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
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/72278/diff/2/?file=2216459#file2216459line1306>
> >
> > 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
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/72278/#comment308453>
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
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> 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
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> 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
Posted by Madhan Neethiraj <ma...@apache.org>.
> 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
Posted by Karthik Manamcheri via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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