xushiyan commented on code in PR #7241:
URL: https://github.com/apache/hudi/pull/7241#discussion_r1032912548
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieTimeline.java:
##########
@@ -335,7 +337,18 @@ public interface HoodieTimeline extends Serializable {
/**
* @return Get the stream of completed instants
*/
- Stream<HoodieInstant> getInstants();
+ default Stream<HoodieInstant> getInstantsAsStream() {
+ return getInstants().stream();
+ }
+
+ /**
+ * @return Get tht list of instants
+ */
+ List<HoodieInstant> getInstants();
+
+ default List<HoodieInstant> getAndCopyInstants() {
+ return new ArrayList<>(getInstants());
+ }
Review Comment:
ditto
also we should use `new ArrayList<>` for `getInstants()` and never expose
`instants` to users. so we should just have `getInstants`, and make javadoc
clear about the behavior and usage
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieTimeline.java:
##########
@@ -335,7 +337,18 @@ public interface HoodieTimeline extends Serializable {
/**
* @return Get the stream of completed instants
*/
- Stream<HoodieInstant> getInstants();
+ default Stream<HoodieInstant> getInstantsAsStream() {
+ return getInstants().stream();
+ }
Review Comment:
can we follow the existing convention to keep default impl. in
HoodieDefaultTimeline? `getInstants()` is not implemented at this level hence
`getInstantsAsStream()` may not be optimized to directly call `stream()`; for
e.g. there could be a timeline subclass which is backed by stream of instants
so you can return the stream directly instead of `getInstants().stream()`.
let's leave the impl. to subclasses
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
