TheNeuralBit commented on a change in pull request #16252:
URL: https://github.com/apache/beam/pull/16252#discussion_r770937703
##########
File path:
sdks/java/harness/src/test/java/org/apache/beam/fn/harness/CachesTest.java
##########
@@ -109,15 +121,27 @@ public void testNestedSubCaches() {
childOfChild.remove("keyB");
assertEquals("childB", child.peek("keyB"));
assertNull(childOfChild.peek("keyB"));
+ }
+
+ @Test
+ public void testClearableCache() {
+ ClearableCache<String, String> cache = new
ClearableCache<>(Caches.eternal());
+ testCache(cache);
+ }
- // Test that clearing the middle cache impacts children but not parent
- parent.put("keyA", "parentA");
- parent.put("keyB", "parentB");
- child.clear();
- assertThat(child.keys(), is(emptyIterable()));
- assertThat(childOfChild.keys(), is(emptyIterable()));
- assertEquals("parentA", parent.peek("keyA"));
- assertEquals("parentB", parent.peek("keyB"));
Review comment:
Is it not possible to have a clearable subcache anymore?
##########
File path:
sdks/java/harness/src/test/java/org/apache/beam/fn/harness/CachesTest.java
##########
@@ -33,32 +34,53 @@
@RunWith(JUnit4.class)
public class CachesTest {
@Test
- public void testNoopCache() {
+ public void testNoopCache() throws Exception {
Cache<String, String> cache = Caches.noop();
cache.put("key", "value");
assertNull(cache.peek("key"));
assertEquals("value", cache.computeIfAbsent("key", (unused) -> "value"));
assertNull(cache.peek("key"));
- assertThat(cache.keys(), is(emptyIterable()));
}
@Test
- public void testEternalCache() {
+ public void testShrinkableIsShrunk() throws Exception {
+ Shrinkable<Object> shrinkable =
+ new Shrinkable<Object>() {
+
+ @Override
+ public Object shrink() {
+ return "wasShrunk";
+ }
+ };
+
+ Cache<Object, Object> cache =
+ Caches.forCache(new
ShrinkOnEviction(CacheBuilder.newBuilder().maximumSize(1)).getCache());
Review comment:
nit: would it be possible to create this through a public API rather
than opening up `forCache`? It used to be private
##########
File path:
sdks/java/fn-execution/src/main/java/org/apache/beam/sdk/fn/stream/DataStreams.java
##########
@@ -181,12 +181,14 @@ public DataStreamDecoder(Coder<T> coder,
PrefetchableIterator<ByteString> inputS
this.inbound = new Inbound();
}
- public void seekToNextByteString() {
Review comment:
Is there any concern with changing a public API? Or is this effectively
`@Internal` since it's in fn-execution?
--
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]
