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 2020/08/11 16:24:40 UTC
[GitHub] [iceberg] RussellSpitzer opened a new pull request #1322: ExpireSnapshots executeWith renamed executeDeleteWith
RussellSpitzer opened a new pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322
The executor passed in executeWith is only used for deletes so we will rename it
to executeDeleteWith. The JavaDoc already states it will only effect deletes and
the implementation already match this name so no other changes are needed.
cc @rdblue + @aokolnychyi Like we discussed, just a rename refactor as we discussed in https://github.com/apache/iceberg/pull/1264
----------------------------------------------------------------
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.
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 a change in pull request #1322: ExpireSnapshots executeWith renamed executeDeleteWith
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322#discussion_r468724418
##########
File path: api/src/main/java/org/apache/iceberg/ExpireSnapshots.java
##########
@@ -96,7 +96,7 @@
* @param executorService an executor service to parallelize tasks to delete manifests and data files
* @return this for method chaining
*/
- ExpireSnapshots executeWith(ExecutorService executorService);
Review comment:
You're right! It is only in master and not 0.9.0: https://github.com/apache/iceberg/commit/ceb6533a593989a86c78277205c0c83108104ded
Nevermind, it should be fine to rename.
----------------------------------------------------------------
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.
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 a change in pull request #1322: ExpireSnapshots executeWith renamed executeDeleteWith
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322#discussion_r468724305
##########
File path: api/src/main/java/org/apache/iceberg/ExpireSnapshots.java
##########
@@ -96,7 +96,7 @@
* @param executorService an executor service to parallelize tasks to delete manifests and data files
* @return this for method chaining
*/
- ExpireSnapshots executeWith(ExecutorService executorService);
Review comment:
It seems we did not release it yet.
----------------------------------------------------------------
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.
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] RussellSpitzer commented on pull request #1322: ExpireSnapshots executeWith renamed executeDeleteWith
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322#issuecomment-672123099
:+1:
On Tue, Aug 11, 2020 at 12:21 PM Ryan Blue <no...@github.com> wrote:
> Thanks, @RussellSpitzer <https://github.com/RussellSpitzer>!
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/iceberg/pull/1322#issuecomment-672110999>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AADE2YNQY6MNFBPVQUWPNILSAF4YRANCNFSM4P3HBJBQ>
> .
>
----------------------------------------------------------------
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.
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 #1322: ExpireSnapshots executeWith renamed executeDeleteWith
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322
----------------------------------------------------------------
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.
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 #1322: ExpireSnapshots executeWith renamed executeDeleteWith
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322#issuecomment-672110999
Thanks, @RussellSpitzer!
----------------------------------------------------------------
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.
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] RussellSpitzer commented on a change in pull request #1322: ExpireSnapshots executeWith renamed executeDeleteWith
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322#discussion_r468720195
##########
File path: api/src/main/java/org/apache/iceberg/ExpireSnapshots.java
##########
@@ -96,7 +96,7 @@
* @param executorService an executor service to parallelize tasks to delete manifests and data files
* @return this for method chaining
*/
- ExpireSnapshots executeWith(ExecutorService executorService);
Review comment:
Ah I didn't think we hit a release boundary since that addition, sure no problem
----------------------------------------------------------------
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.
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 a change in pull request #1322: ExpireSnapshots executeWith renamed executeDeleteWith
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1322:
URL: https://github.com/apache/iceberg/pull/1322#discussion_r468711615
##########
File path: api/src/main/java/org/apache/iceberg/ExpireSnapshots.java
##########
@@ -96,7 +96,7 @@
* @param executorService an executor service to parallelize tasks to delete manifests and data files
* @return this for method chaining
*/
- ExpireSnapshots executeWith(ExecutorService executorService);
Review comment:
Can we add the new name in parallel and deprecate this name? We should be careful about changes to the public API and give people at least a release to switch over before removing public methods.
----------------------------------------------------------------
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.
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