You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "singhpk234 (via GitHub)" <gi...@apache.org> on 2023/04/25 16:07:24 UTC
[GitHub] [iceberg] singhpk234 opened a new pull request, #7429: Spark: Refactor SparkReadConf use primitive type for confs with default values
singhpk234 opened a new pull request, #7429:
URL: https://github.com/apache/iceberg/pull/7429
### About Change
Address review feedback https://github.com/apache/iceberg/pull/7422#discussion_r1175813062 .
cc @aokolnychyi
--
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] aokolnychyi commented on pull request #7429: Spark: Refactor SparkReadConf use primitive type for confs with default values
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on PR #7429:
URL: https://github.com/apache/iceberg/pull/7429#issuecomment-1526025783
Thanks, @singhpk234!
--
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 pull request #7429: Spark: Refactor SparkReadConf use primitive type for confs with default values
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on PR #7429:
URL: https://github.com/apache/iceberg/pull/7429#issuecomment-1526171169
Thanks @singhpk234 ! i was thinkiing about @szehon-ho point , I don't think we can set an expectation that people can extend SparkReadConf and have signatures align. So overall, this change looks good to 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] jackye1995 commented on pull request #7429: Spark: Refactor SparkReadConf use primitive type for confs with default values
Posted by "jackye1995 (via GitHub)" <gi...@apache.org>.
jackye1995 commented on PR #7429:
URL: https://github.com/apache/iceberg/pull/7429#issuecomment-1524641409
> is it a backward compatible change?
Under what case would user have this error?
--
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] szehon-ho commented on pull request #7429: Spark: Refactor SparkReadConf use primitive type for confs with default values
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #7429:
URL: https://github.com/apache/iceberg/pull/7429#issuecomment-1526019941
Yea I am thinking, if our caller does not recompile their library on newer Iceberg version , they will hit it, but I think its not the normal flow. Thanks for double checking.
--
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] singhpk234 commented on pull request #7429: Spark: Refactor SparkReadConf use primitive type for confs with default values
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #7429:
URL: https://github.com/apache/iceberg/pull/7429#issuecomment-1525903089
we can get the values in both long, Long, so using the function call directly would not be problematic but let's say if some one is extending SparkReadConf and over-riding the `streamFromTimestamp()` might face issue as now the signatures don't match. But it seems like a very niche use case.
Happy to know your thoughts and make the changes accordingly, as `streamFromTimestamp()` is the only released public method will mark it deprecated, and add a new one with primitive type 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] aokolnychyi merged pull request #7429: Spark: Refactor SparkReadConf use primitive type for confs with default values
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi merged PR #7429:
URL: https://github.com/apache/iceberg/pull/7429
--
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