aiborodin commented on code in PR #14406:
URL: https://github.com/apache/iceberg/pull/14406#discussion_r2587876861
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/TableMetadataCache.java:
##########
@@ -80,6 +87,14 @@ ResolvedSchemaInfo schema(TableIdentifier identifier, Schema
input) {
return schema(identifier, input, true);
}
+ ResolvedSchemaInfo getResolvedSchemaInfo(TableIdentifier identifier, Schema
input) {
+ CacheItem tableCacheItem = tableCache.get(identifier);
+ if (tableCacheItem == null) {
+ return null;
+ }
+ return tableCacheItem.inputSchemas.get(input);
+ }
+
Review Comment:
I replaced it with existing `@VisibleForTesting` methods in this PR.
Although I don't like these methods and would prefer hiding internal state
behind simple `get()` calls.
We can do this in a separate PR.
I originally added this method to allow retrieving a cached schema without
caching it if not present (which `TableMetadataCache.schema()` always does).
Currently, this is only possible using "get internal cache" methods.
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/TableMetadataCache.java:
##########
@@ -50,13 +51,19 @@ class TableMetadataCache {
private final Catalog catalog;
private final long refreshMs;
+ private final Clock cacheRefreshClock;
private final int inputSchemasPerTableCacheMaximumSize;
private final Map<TableIdentifier, CacheItem> tableCache;
TableMetadataCache(
- Catalog catalog, int maximumSize, long refreshMs, int
inputSchemasPerTableCacheMaximumSize) {
+ Catalog catalog,
+ int maximumSize,
+ long refreshMs,
+ Clock cacheRefreshClock,
Review Comment:
It is a common pattern to inject a `Clock` as a dependency in Java (used for
both prod and testing). Quoting [Java
documentation](https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/time/Clock.html):
> Best practice for applications is to pass a Clock into any method that
requires the current instant and time-zone. A dependency injection framework is
one way to achieve this:
>
> ```java
> public class MyBean {
> private Clock clock; // dependency inject
> ...
> public void process(LocalDate eventDate) {
> if (eventDate.isBefore(LocalDate.now(clock)) {
> ...
> }
> }
> }
> ```
> This approach allows an alternative clock, such as
[fixed](https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/time/Clock.html#fixed(java.time.Instant,java.time.ZoneId))
or
[offset](https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/time/Clock.html#offset(java.time.Clock,java.time.Duration))
to be used during testing.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
