You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Suma Shivaprasad <su...@gmail.com> on 2016/04/19 23:29:34 UTC
Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/
-----------------------------------------------------------
Review request for atlas.
Bugs: ATLAS-583
https://issues.apache.org/jira/browse/ATLAS-583
Repository: atlas
Description
-------
Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
Diffs
-----
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
Diff: https://reviews.apache.org/r/46409/diff/
Testing
-------
Added tests in HiveHookIT
Thanks,
Suma Shivaprasad
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Hemanth Yamijala <yh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/#review129864
-----------------------------------------------------------
Fix it, then Ship it!
Suma, as you had mentioned offline, you can remove the public getResource method in AtlasClient and commit this. Other changes are fine by me. +1
client/src/main/java/org/apache/atlas/AtlasClient.java (line 305)
<https://reviews.apache.org/r/46409/#comment193414>
As discussed offline, can you please revert this, as it no longer seems used?
- Hemanth Yamijala
On April 20, 2016, 10:01 p.m., Suma Shivaprasad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46409/
> -----------------------------------------------------------
>
> (Updated April 20, 2016, 10:01 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-583
> https://issues.apache.org/jira/browse/ATLAS-583
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
>
>
> Diffs
> -----
>
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
> addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
> client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
>
> Diff: https://reviews.apache.org/r/46409/diff/
>
>
> Testing
> -------
>
> Added tests in HiveHookIT
>
>
> Thanks,
>
> Suma Shivaprasad
>
>
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/
-----------------------------------------------------------
(Updated April 21, 2016, 11:10 p.m.)
Review request for atlas.
Bugs: ATLAS-583
https://issues.apache.org/jira/browse/ATLAS-583
Repository: atlas
Description
-------
Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
Diffs (updated)
-----
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
Diff: https://reviews.apache.org/r/46409/diff/
Testing
-------
Added tests in HiveHookIT
Thanks,
Suma Shivaprasad
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/
-----------------------------------------------------------
(Updated April 20, 2016, 10:01 p.m.)
Review request for atlas.
Bugs: ATLAS-583
https://issues.apache.org/jira/browse/ATLAS-583
Repository: atlas
Description
-------
Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
Diffs (updated)
-----
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
Diff: https://reviews.apache.org/r/46409/diff/
Testing
-------
Added tests in HiveHookIT
Thanks,
Suma Shivaprasad
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/
-----------------------------------------------------------
(Updated April 20, 2016, 9:59 p.m.)
Review request for atlas.
Bugs: ATLAS-583
https://issues.apache.org/jira/browse/ATLAS-583
Repository: atlas
Description
-------
Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
Diffs (updated)
-----
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
Diff: https://reviews.apache.org/r/46409/diff/
Testing
-------
Added tests in HiveHookIT
Thanks,
Suma Shivaprasad
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/
-----------------------------------------------------------
(Updated April 20, 2016, 9:54 p.m.)
Review request for atlas.
Changes
-------
Fixed review comments
Bugs: ATLAS-583
https://issues.apache.org/jira/browse/ATLAS-583
Repository: atlas
Description
-------
Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
Diffs (updated)
-----
addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
Diff: https://reviews.apache.org/r/46409/diff/
Testing
-------
Added tests in HiveHookIT
Thanks,
Suma Shivaprasad
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Suma Shivaprasad <su...@gmail.com>.
> On April 20, 2016, 4:36 a.m., Hemanth Yamijala wrote:
> > addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java, line 421
> > <https://reviews.apache.org/r/46409/diff/1/?file=1351748#file1351748line421>
> >
> > For my understanding, can you please explain why this step is necessary. It looks like our change sequence is:
> >
> > * Update DB / Table entity - retaining the old name of table.
> > * Update Storage Descriptor / Columns.
> > * Partial update table with new name.
> >
> > I am trying to understand why Step 1 is required. Is it to take care of situations where the old table did not exist before in Atlas?
>
> Suma Shivaprasad wrote:
> Yes its required to create old table state when it doesnt exist
Keeping it open in case you have any further comments/questions
- Suma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/#review129681
-----------------------------------------------------------
On April 20, 2016, 9:54 p.m., Suma Shivaprasad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46409/
> -----------------------------------------------------------
>
> (Updated April 20, 2016, 9:54 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-583
> https://issues.apache.org/jira/browse/ATLAS-583
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
>
>
> Diffs
> -----
>
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
> addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
> client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
>
> Diff: https://reviews.apache.org/r/46409/diff/
>
>
> Testing
> -------
>
> Added tests in HiveHookIT
>
>
> Thanks,
>
> Suma Shivaprasad
>
>
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Suma Shivaprasad <su...@gmail.com>.
> On April 20, 2016, 4:36 a.m., Hemanth Yamijala wrote:
> > addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java, line 421
> > <https://reviews.apache.org/r/46409/diff/1/?file=1351748#file1351748line421>
> >
> > For my understanding, can you please explain why this step is necessary. It looks like our change sequence is:
> >
> > * Update DB / Table entity - retaining the old name of table.
> > * Update Storage Descriptor / Columns.
> > * Partial update table with new name.
> >
> > I am trying to understand why Step 1 is required. Is it to take care of situations where the old table did not exist before in Atlas?
Yes its required to create old table state when it doesnt exist
- Suma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/#review129681
-----------------------------------------------------------
On April 19, 2016, 9:29 p.m., Suma Shivaprasad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46409/
> -----------------------------------------------------------
>
> (Updated April 19, 2016, 9:29 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-583
> https://issues.apache.org/jira/browse/ATLAS-583
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
>
>
> Diffs
> -----
>
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
> addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
> client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
>
> Diff: https://reviews.apache.org/r/46409/diff/
>
>
> Testing
> -------
>
> Added tests in HiveHookIT
>
>
> Thanks,
>
> Suma Shivaprasad
>
>
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Suma Shivaprasad <su...@gmail.com>.
> On April 20, 2016, 4:36 a.m., Hemanth Yamijala wrote:
> > addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java, line 422
> > <https://reviews.apache.org/r/46409/diff/1/?file=1351748#file1351748line422>
> >
> > We are resetting the table's name to older one. But the DB instance added to entities would still be a new DB name only. Is that a problem?
It wouldnt cause an issue in the normal flow but could cause it be inconsistent if the following updates to replace the table name, column, sd fail. So fixed it by replacing with old DB instance
- Suma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/#review129681
-----------------------------------------------------------
On April 20, 2016, 9:54 p.m., Suma Shivaprasad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46409/
> -----------------------------------------------------------
>
> (Updated April 20, 2016, 9:54 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-583
> https://issues.apache.org/jira/browse/ATLAS-583
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
>
>
> Diffs
> -----
>
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
> addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
> client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
>
> Diff: https://reviews.apache.org/r/46409/diff/
>
>
> Testing
> -------
>
> Added tests in HiveHookIT
>
>
> Thanks,
>
> Suma Shivaprasad
>
>
Re: Review Request 46409: ATLAS-583 Rename table should retain column,
partition columns tags
Posted by Hemanth Yamijala <yh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46409/#review129681
-----------------------------------------------------------
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java (line 419)
<https://reviews.apache.org/r/46409/#comment193188>
For my understanding, can you please explain why this step is necessary. It looks like our change sequence is:
* Update DB / Table entity - retaining the old name of table.
* Update Storage Descriptor / Columns.
* Partial update table with new name.
I am trying to understand why Step 1 is required. Is it to take care of situations where the old table did not exist before in Atlas?
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java (line 420)
<https://reviews.apache.org/r/46409/#comment193189>
We are resetting the table's name to older one. But the DB instance added to entities would still be a new DB name only. Is that a problem?
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java (line 436)
<https://reviews.apache.org/r/46409/#comment193190>
This fragment looks similar to one with cols. Can we extract a common method?
addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java (line 445)
<https://reviews.apache.org/r/46409/#comment193191>
Can this not be done along with the earlier loop where we reset column names? I see a couple of points that seem a little hard to understand with the current model:
* We iterate over a different collection of columns between here and the earlier loop. But getAllCols is just a collection of both cols and partition keys. So, it should be identical to cols and partCols in previous loops.
* We are running the loop twice, which I feel can be avoided.
* We are constructing the old column QF names twice again.
So, can we say:
for (Referenceable col : cols) {
// reset the name to old name
// create a new column entity with partial update using old name as key and new name as value.
// Add new entity to list
}
addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java (line 670)
<https://reviews.apache.org/r/46409/#comment193192>
Not really a part of this JIRA, but still it is useful to move this method to AtlasClient itself, IMO. If its a minor change. That way, we probably don't need to expose getResource as well.
addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java (line 687)
<https://reviews.apache.org/r/46409/#comment193193>
Same applies for this API as well.
- Hemanth Yamijala
On April 19, 2016, 9:29 p.m., Suma Shivaprasad wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46409/
> -----------------------------------------------------------
>
> (Updated April 19, 2016, 9:29 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-583
> https://issues.apache.org/jira/browse/ATLAS-583
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Fixed it by first renaming the columns, sd, partition keys and then renaming the table so that the columns, sd, etc are not deleted/recreated again becaused they are deduped based on their QualifiedName which has tableName as part of it.
>
>
> Diffs
> -----
>
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java a28b4ac
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/hook/HiveHook.java 749294f
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/model/HiveDataModelGenerator.java f099e39
> addons/hive-bridge/src/test/java/org/apache/atlas/hive/hook/HiveHookIT.java 4c7ac70
> client/src/main/java/org/apache/atlas/AtlasClient.java 22a1726
>
> Diff: https://reviews.apache.org/r/46409/diff/
>
>
> Testing
> -------
>
> Added tests in HiveHookIT
>
>
> Thanks,
>
> Suma Shivaprasad
>
>