You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/02/17 07:47:34 UTC

[GitHub] [iceberg] happyprg opened a new pull request #4151: fix: rename variable name partition to partition_spec in python-api-intro.md

happyprg opened a new pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151


   closed #4150 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdblue merged pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809624175



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       @dramaticlly 
   Thank you for giving me good direction.
   I changed text `tmpdir` to `{tmpdir}`. [following current convention(replacement text)](https://github.com/apache/iceberg/pull/4151/files#diff-e2fa3022513566d7037734d0c167b142be6f86df26aa59fe5548eff6182dd15bL39)  
   I think it is more simple and keep following current convention, how about this?
   
   Firstly, I tried resolving `tmpdir` using [docs.python.org/3/library/tempfile.html#module-tempfile](https://docs.python.org/3/library/tempfile.html#module-tempfile), but I failed.
   ![스크린샷 2022-02-18 오전 11 24 33](https://user-images.githubusercontent.com/779556/154605733-d69ccd92-0d2f-461a-9e94-0538822f1094.png)
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809624175



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       I changed text `tmpdir` to `{tmpdir}`. [following current convention(replacement text)](https://github.com/apache/iceberg/pull/4151/files#diff-e2fa3022513566d7037734d0c167b142be6f86df26aa59fe5548eff6182dd15bL39) 
   
   how about this?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809624175



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       @dramaticlly 
   Thank you for giving me good direction.
   I changed text `tmpdir` to `{tmpdir}`. [following current convention(replacement text)](https://github.com/apache/iceberg/pull/4151/files#diff-e2fa3022513566d7037734d0c167b142be6f86df26aa59fe5548eff6182dd15bL39) 
   
   how about this?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdblue commented on pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#issuecomment-1046280307


   Thanks, @happyprg!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] dramaticlly commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
dramaticlly commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809612250



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       yeah I guess the legacy python API asked to supply the valid hive metastore warehouse directory and fails the code if default to None. But I guess `tmpdir` is not resolved directly, if needs proper setup I guess need to use the python module https://docs.python.org/3/library/tempfile.html#module-tempfile. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] dramaticlly commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
dramaticlly commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r810306367



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       but I think PMC is needed to merge your code. CC @jun-he 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809624175



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       @dramaticlly 
   Thank you for giving me good direction.
   I changed text `tmpdir` to `{tmpdir}`. [following current convention(replacement text)](https://github.com/apache/iceberg/pull/4151/files#diff-e2fa3022513566d7037734d0c167b142be6f86df26aa59fe5548eff6182dd15bL39)  
   I think it is more simple and keep following current convention, how about this?
   
   Firstly, I tried resolving `tmpdir` using [docs.python.org/3/library/tempfile.html#module-tempfile](https://docs.python.org/3/library/tempfile.html#module-tempfile), but I failed.
   ![스크린샷 2022-02-18 오전 11 24 33](https://user-images.githubusercontent.com/779556/154605733-d69ccd92-0d2f-461a-9e94-0538822f1094.png)
   and If I want to fix it, it is 
   
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] dramaticlly commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
dramaticlly commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809444874



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       curious to know, why add this line as well ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdblue commented on pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#issuecomment-1047267442


   Sorry, I thought I already had. Thanks for pinging me.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#issuecomment-1046751435


   > Thanks, @happyprg!
   
   Thank you for approving my PR @rdblue @dramaticlly 
   May I ask you the schedule for merging this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809602266



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       > curious to know, why add this line as well ?
   
   @dramaticlly 
   Thank you for checking my PR.
   I thought that When I setup `hive.metastore.uris` property, Warehouse location is required mandatory.
   I checked test case codes of Iceberg [here](https://github.com/apache/iceberg/blob/master/python_legacy/tests/hive/test_hive_tables.py#L111) and [here](https://github.com/apache/iceberg/blob/master/python_legacy/tests/hive/test_hive_tables.py#L131) also.
   ![스크린샷 2022-02-18 오전 10 15 00](https://user-images.githubusercontent.com/779556/154598990-b6e0b759-5da7-439a-b639-8ae567f696a3.png)
   ![스크린샷 2022-02-18 오전 10 15 12](https://user-images.githubusercontent.com/779556/154598987-f12220a1-b6ce-4e41-9826-b1fb481ef73b.png)
   ![스크린샷 2022-02-18 오전 10 15 23](https://user-images.githubusercontent.com/779556/154598979-408c463a-531a-4de0-b53a-05b88e58e703.png)
   
   Am I misunderstanding?
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809602266



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       > curious to know, why add this line as well ?
   
   @dramaticlly 
   Thank you for checking my PR.
   I thought that When I setup `hive.metastore.uris` property, Warehouse location is required mandatory.
   I checked [test case codes of Iceberg](https://github.com/apache/iceberg/blob/master/python_legacy/tests/hive/test_hive_tables.py#L131) also.
   ![스크린샷 2022-02-18 오전 10 15 00](https://user-images.githubusercontent.com/779556/154598990-b6e0b759-5da7-439a-b639-8ae567f696a3.png)
   ![스크린샷 2022-02-18 오전 10 15 12](https://user-images.githubusercontent.com/779556/154598987-f12220a1-b6ce-4e41-9826-b1fb481ef73b.png)
   ![스크린샷 2022-02-18 오전 10 15 23](https://user-images.githubusercontent.com/779556/154598979-408c463a-531a-4de0-b53a-05b88e58e703.png)
   
   Am I misunderstanding?
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] happyprg commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
happyprg commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r809602266



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       > curious to know, why add this line as well ?
   
   @dramaticlly 
   Thank you for checking my PR.
   I thought that When I setup `hive.metastore.uris` property, Warehouse location is required mandatory.
   I checked test case codes of Iceberg [here](https://github.com/apache/iceberg/blob/master/python_legacy/tests/hive/test_hive_tables.py#L111) and [here](https://github.com/apache/iceberg/blob/master/python_legacy/tests/hive/test_hive_tables.py#L131) also.
   ![스크린샷 2022-02-18 오전 10 15 00](https://user-images.githubusercontent.com/779556/154598990-b6e0b759-5da7-439a-b639-8ae567f696a3.png)
   
   ![스크린샷 2022-02-18 오전 10 15 23](https://user-images.githubusercontent.com/779556/154598979-408c463a-531a-4de0-b53a-05b88e58e703.png)
   
   Am I misunderstanding?
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] dramaticlly commented on a change in pull request #4151: Docs: update python-api-intro.md

Posted by GitBox <gi...@apache.org>.
dramaticlly commented on a change in pull request #4151:
URL: https://github.com/apache/iceberg/pull/4151#discussion_r810304010



##########
File path: docs/versioned/api/python-api-intro.md
##########
@@ -36,7 +36,8 @@ To create a catalog:
 from iceberg.hive import HiveTables
 
 # instantiate Hive Tables
-conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}'}
+conf = {"hive.metastore.uris": 'thrift://{hms_host}:{hms_port}',
+        "hive.metastore.warehouse.dir": tmpdir}

Review comment:
       yes I think that's fine. It's for README.md so i guess it's ok. LGTM for the change 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org