You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "amogh-jahagirdar (via GitHub)" <gi...@apache.org> on 2023/01/30 16:20:42 UTC

[GitHub] [iceberg] amogh-jahagirdar opened a new pull request, #6704: Python: Update pyproject.toml to include dev folder.

amogh-jahagirdar opened a new pull request, #6704:
URL: https://github.com/apache/iceberg/pull/6704

   Python: Update pyproject.toml to include dev folder. 
   
   Currently, some files which are required for during release validation such as 
   
   1.) .rat-excludes
   2.) scripts in the dev/ folder for running integration tests
   
   are not included in the source release. This change includes all the files in the dev folder


-- 
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] amogh-jahagirdar commented on a diff in pull request #6704: Python: Update pyproject.toml to include dev folder.

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #6704:
URL: https://github.com/apache/iceberg/pull/6704#discussion_r1090854455


##########
python/pyproject.toml:
##########
@@ -39,11 +39,13 @@ packages = [
     { from = "vendor", include = "fb303" },
     { from = "vendor", include = "hive_metastore" },
     { include = "tests", format = "sdist" },
-    { include = "dev/check-license", format = "sdist" },
     { include = "Makefile", format = "sdist" },
     { include = "NOTICE", format = ["sdist", "wheel"] }
 ]
 
+include = [
+    {path = "dev", format = "sdist"}
+]

Review Comment:
   Made a separate list instead of adding in packages which seems to be what https://python-poetry.org/docs/pyproject/#include-and-exclude suggests.
   
   When I tried adding the glob "dev/*" to the existing packages list I would always get errors saying that the files were not packages which makes sense, but when I add the files individually it works.
   
   ```
   .rat-excludes is not a package.
   ```
   
    Since we want to include everything in the dev folder, to make it easier in the future I went this route.
   
   Technically we can add the Makefile, NOTICE files as well but I didn't want to make this change more intrusive before release.



-- 
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] amogh-jahagirdar commented on a diff in pull request #6704: Python: Update pyproject.toml to include dev folder.

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #6704:
URL: https://github.com/apache/iceberg/pull/6704#discussion_r1090854455


##########
python/pyproject.toml:
##########
@@ -39,11 +39,13 @@ packages = [
     { from = "vendor", include = "fb303" },
     { from = "vendor", include = "hive_metastore" },
     { include = "tests", format = "sdist" },
-    { include = "dev/check-license", format = "sdist" },
     { include = "Makefile", format = "sdist" },
     { include = "NOTICE", format = ["sdist", "wheel"] }
 ]
 
+include = [
+    {path = "dev", format = "sdist"}
+]

Review Comment:
   Made a separate list instead of adding in packages which seems to be what https://python-poetry.org/docs/pyproject/#include-and-exclude suggests.
   
   When I tried adding the glob "dev/*" to the existing packages list I would always get errors saying that the files were not packages which makes sense, but when I add the files individually it works. Since we want to include everything in the dev folder, to make it easier in the future I went this route.
   
   Technically we can add the Makefile, NOTICE files as well but I didn't want to make this change more intrusive before release.



-- 
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] amogh-jahagirdar commented on a diff in pull request #6704: Python: Update pyproject.toml to include dev folder.

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #6704:
URL: https://github.com/apache/iceberg/pull/6704#discussion_r1090854455


##########
python/pyproject.toml:
##########
@@ -39,11 +39,13 @@ packages = [
     { from = "vendor", include = "fb303" },
     { from = "vendor", include = "hive_metastore" },
     { include = "tests", format = "sdist" },
-    { include = "dev/check-license", format = "sdist" },
     { include = "Makefile", format = "sdist" },
     { include = "NOTICE", format = ["sdist", "wheel"] }
 ]
 
+include = [
+    {path = "dev", format = "sdist"}
+]

Review Comment:
   Made a separate list instead of adding in packages which seems to be what https://python-poetry.org/docs/pyproject/#include-and-exclude suggests.
   
   When I tried adding the glob "dev/*" to the existing packages list I would always get errors saying that the files were not packages which makes sense. 
   
   ```
   .rat-excludes is not a package.
   ```
   
   However, when I add the files individually it works.
   
    Since we want to include everything in the dev folder, to make it easier in the future I went this route.
   
   Technically we can add the Makefile, NOTICE files as well but I didn't want to make this change more intrusive before release.



-- 
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] Fokko commented on pull request #6704: Python: Update pyproject.toml to include dev folder.

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #6704:
URL: https://github.com/apache/iceberg/pull/6704#issuecomment-1409963309

   Thanks @amogh-jahagirdar for fixing this, and @jackye1995 and @singhpk234  for the review 👏🏻 


-- 
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] Fokko merged pull request #6704: Python: Update pyproject.toml to include dev folder.

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #6704:
URL: https://github.com/apache/iceberg/pull/6704


-- 
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