sjvanrossum commented on code in PR #34135: URL: https://github.com/apache/beam/pull/34135#discussion_r1979614963
########## sdks/python/apache_beam/io/gcp/bigquery_tools.py: ########## @@ -351,6 +352,9 @@ class BigQueryWrapper(object): TEMP_DATASET = 'beam_temp_dataset_' HISTOGRAM_METRIC_LOGGER = MetricLogger() + + # Default TTL for cached table definitions in seconds + DEFAULT_TABLE_DEFINITION_TTL = 3600 # 1 hour Review Comment: > from cachetools import TTLCache from cachetools.keys import hashkey from functools import partial #Use thread-safe TTLCache with proper key function self.table_cache = TTLCache( maxsize=self.DEFAULT_CACHE_MAX_SIZE, ttl=self.DEFAULT_TABLE_DEFINITION_TTL, getsizeof=None) Have another look at the comment that you responded to a couple minutes ago, since this suggestion doesn't address the issue with the lifetime scope of the cache container yet. Essentially, the cache should not be defined as an instance variable and either cachetools or functools should be sufficient, I don't think we have to mix and match them. -- 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: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
