Repository navigation
Conversation
|
@aokolnychyi You were reviewing #9029, could you review this one as well please? |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
|
Can it be reopened? The PR is ready to review. |
| import java.io.ByteArrayOutputStream; | ||
| import java.io.ObjectInputStream; | ||
| import java.io.ObjectOutputStream; | ||
| import org.junit.Test; |
There was a problem hiding this comment.
| import org.junit.Test; | |
| import org.junit.jupiter.api.Test; |
| * | ||
| * @param <T> the type of the result | ||
| */ | ||
| public static class Try<T> implements Serializable { |
There was a problem hiding this comment.
Instead of using a util class what do you think about adding a static helper like:
public static <T> Try<T> capture(ThrowingSupplier<T> supplier) {
try {
return success(supplier.get());
} catch (Exception e) {
return failure(e);
}
}
Then usage would look like
Try<LocationProvider> lp = Try.capture(table::locationProvider);
| if (lazyLocationProvider == null) { | ||
| this.lazyLocationProvider = LocationProviders.locationsFor(location, properties); | ||
| } | ||
| try { |
There was a problem hiding this comment.
We can move this logic into the Try class as public T getOrThrow()
| @@ -265,14 +267,15 @@ public EncryptionManager encryption() { | |||
|
|
|||
| @Override | |||
| public LocationProvider locationProvider() { | |||
There was a problem hiding this comment.
Can we add some tests in spark to ensure that the exception isn't thrown on instantiation but when serializableTable.locationProvider() is thrown
- Move Try class from TryUtil.java to standalone Try.java - Update imports in SerializableTable.java and test files - Rename TestTryUtil.java to TestTry.java - Address reviewer feedback from PR apache#12564 - Maintain same API: Try.capture() for deferred exception handling
- Move Try class from TryUtil.java to standalone Try.java - Update imports in SerializableTable.java and test files - Rename TestTryUtil.java to TestTry.java - Address reviewer feedback from PR apache#12564 - Maintain same API: Try.capture() for deferred exception handling
|
I created a new PR #14280 as this one is closed already. |
…ble apache#12564 (apache#14280) (cherry picked from commit e667f64)
2 participants
Closes #11527 in a way that does not break fixes provided by changes needed to fix #9029.
It eagerly calculates the location provider field, but wraps that into a newly implemented wrapper
TryUtil.Trythat only throws an error if code really needs to use that location provider. In case table needs to be read only by other processes the error is not thrown, even if there is an error with getting the location provider object.