Re: Review Request 52166: Allow optional attributes to be removed when updating type

2016-09-27 Thread Shwetha GS


> On Sept. 23, 2016, 6:19 a.m., Shwetha GS wrote:
> > In case the attribute is added again, the data will be wrong. We can't 
> > discount that as an edge case, as it causes data inconsistency. Follow up 
> > jiras will hardly be taken up. Its fine even if we decide that we don't 
> > allow adding the attribute again(the deleted attribute). But we should 
> > address the issue completely
> > 
> > Add UTs in all your patches. Just an end to end test for this patch now 
> > will not ensure that this functionality won't be broken in further commits
> 
> Suma Shivaprasad wrote:
> One sugestion to resolve this case would be
> 
> 1. Add a isDeleted flag to the attribute. It may be useful to know that 
> this atribute was deleted and in case of upgrade/rollbacks , we could 
> potentially use this flag to resurface it back again and roll back the type 
> update.
> 2. Rename the existing property key - 
> http://s3.thinkaurelius.com/docs/titan/0.5.1/schema.html#_changing_schema_elements.
>  Not sure how much time this operation would take. If its an expensive 
> operation, we would need to run this as an asynchronous thread.

We can add the isDeleted flag for now and can cleanup the deleted attributes 
during maintenance windows like upgrades


- Shwetha


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


On Sept. 22, 2016, 6:22 p.m., Sarath Kumar Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52166/
> ---
> 
> (Updated Sept. 22, 2016, 6:22 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj, Shwetha GS, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1175
> https://issues.apache.org/jira/browse/ATLAS-1175
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> we restrict optional attributes to be removed from the type system. This 
> needs to be allowed
> 
> 
> Diffs
> -
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypeUtils.java 
> f5c2ce9 
> 
> Diff: https://reviews.apache.org/r/52166/diff/
> 
> 
> Testing
> ---
> 
> tested using REST calls to delete optional and required attributes and only 
> optional attributes were removed.
> 
> 
> Thanks,
> 
> Sarath Kumar Subramanian
> 
>



Re: Review Request 52166: Allow optional attributes to be removed when updating type

2016-09-23 Thread Suma Shivaprasad


> On Sept. 23, 2016, 6:19 a.m., Shwetha GS wrote:
> > In case the attribute is added again, the data will be wrong. We can't 
> > discount that as an edge case, as it causes data inconsistency. Follow up 
> > jiras will hardly be taken up. Its fine even if we decide that we don't 
> > allow adding the attribute again(the deleted attribute). But we should 
> > address the issue completely
> > 
> > Add UTs in all your patches. Just an end to end test for this patch now 
> > will not ensure that this functionality won't be broken in further commits

One sugestion to resolve this case would be

1. Add a isDeleted flag to the attribute. It may be useful to know that this 
atribute was deleted and in case of upgrade/rollbacks , we could potentially 
use this flag to resurface it back again and roll back the type update.
2. Rename the existing property key - 
http://s3.thinkaurelius.com/docs/titan/0.5.1/schema.html#_changing_schema_elements.
 Not sure how much time this operation would take. If its an expensive 
operation, we would need to run this as an asynchronous thread.


- Suma


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


On Sept. 22, 2016, 6:22 p.m., Sarath Kumar Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52166/
> ---
> 
> (Updated Sept. 22, 2016, 6:22 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj, Shwetha GS, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1175
> https://issues.apache.org/jira/browse/ATLAS-1175
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> we restrict optional attributes to be removed from the type system. This 
> needs to be allowed
> 
> 
> Diffs
> -
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypeUtils.java 
> f5c2ce9 
> 
> Diff: https://reviews.apache.org/r/52166/diff/
> 
> 
> Testing
> ---
> 
> tested using REST calls to delete optional and required attributes and only 
> optional attributes were removed.
> 
> 
> Thanks,
> 
> Sarath Kumar Subramanian
> 
>



Re: Review Request 52166: Allow optional attributes to be removed when updating type

2016-09-22 Thread Shwetha GS

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



In case the attribute is added again, the data will be wrong. We can't discount 
that as an edge case, as it causes data inconsistency. Follow up jiras will 
hardly be taken up. Its fine even if we decide that we don't allow adding the 
attribute again(the deleted attribute). But we should address the issue 
completely

Add UTs in all your patches. Just an end to end test for this patch now will 
not ensure that this functionality won't be broken in further commits

- Shwetha GS


On Sept. 22, 2016, 6:22 p.m., Sarath Kumar Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52166/
> ---
> 
> (Updated Sept. 22, 2016, 6:22 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj, Shwetha GS, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1175
> https://issues.apache.org/jira/browse/ATLAS-1175
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> we restrict optional attributes to be removed from the type system. This 
> needs to be allowed
> 
> 
> Diffs
> -
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypeUtils.java 
> f5c2ce9 
> 
> Diff: https://reviews.apache.org/r/52166/diff/
> 
> 
> Testing
> ---
> 
> tested using REST calls to delete optional and required attributes and only 
> optional attributes were removed.
> 
> 
> Thanks,
> 
> Sarath Kumar Subramanian
> 
>