You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Guosmilesmile (via GitHub)" <gi...@apache.org> on 2025/04/11 12:58:39 UTC

[PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

Guosmilesmile opened a new pull request, #12773:
URL: https://github.com/apache/iceberg/pull/12773

   This PR fixes two things:
   
   1. Use checkArgument with `newRateLimit`, not rateLimit, as expected.
   2. Add checkNotNull before checking `newRateLimit.toMillis() > 0`.


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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

   Merged to main.
   Thanks for catching and fixing this @Guosmilesmile!
   


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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

   Hey @pvary quick question. I know we usually do a PR against the latest Flink Version and then follow up with backports PR. But of changes as small as this one, do we want to still follow the same procedure? 


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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

   @rodmeneses: I had a longer discussion with @Guosmilesmile to separate the PRs, and I think having more PRs is smaller issue, than having one big PR. Especially in the beginning when we have more comments on the PR, which might cause triple work.
   
   All-in-all, both solutions are fine for this small PRs


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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

   > Hey @pvary quick question. I know we usually do a PR against the latest Flink Version and then follow up with backports PR. But of changes as small as this one, do we want to still follow the same procedure?
   
   Thank you very much for your suggestion. I’m not very familiar with this area yet, if necessary, I will consider merging the changes appropriately next time.


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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

   > Do you have any idea about the CI failure?
   
   Flaky test :(
   See: #11047


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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

   @pvary Hi, Peter! Do you have any idea about the CI failure?  I think this change is unrelated to the Hadoop commit timeout issue.
   
   I thought this situation is related to https://github.com/apache/iceberg/pull/11654, but I’m not sure how to resolve it.
   


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


Re: [PR] Flink: fix rateLimit argument check in TableMaintenance [iceberg]

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

   > Merged to main.
   > Thanks for catching and fixing this @Guosmilesmile!
   > 
   > Could you please create the backport PR?
   > 
   
   @pvary  Peter , thanks for your review! I will create the backport soon.


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