john-bodley commented on code in PR #24373:
URL: https://github.com/apache/superset/pull/24373#discussion_r1272479600
##########
superset/config.py:
##########
@@ -645,8 +645,10 @@ class D3Format(TypedDict, total=False):
THUMBNAIL_CHART_DIGEST_FUNC: Optional[Callable[[Slice, ExecutorType, str],
str]] = None
THUMBNAIL_CACHE_CONFIG: CacheConfig = {
- "CACHE_TYPE": "NullCache",
- "CACHE_NO_NULL_WARNING": True,
+ 'CACHE_TYPE': 'redis',
+ 'CACHE_DEFAULT_TIMEOUT': 24*60*60*7,
Review Comment:
Could you use the `timedelta` instead
([example](https://github.com/apache/superset/blob/b8a3eeffdbfd6db852510ac409e4852fbac2f52c/superset/config.py#L952))
to improve readability.
##########
superset/config.py:
##########
@@ -645,8 +645,10 @@ class D3Format(TypedDict, total=False):
THUMBNAIL_CHART_DIGEST_FUNC: Optional[Callable[[Slice, ExecutorType, str],
str]] = None
THUMBNAIL_CACHE_CONFIG: CacheConfig = {
- "CACHE_TYPE": "NullCache",
- "CACHE_NO_NULL_WARNING": True,
+ 'CACHE_TYPE': 'redis',
Review Comment:
See my previous comment.
##########
superset/config.py:
##########
@@ -1317,7 +1319,7 @@ def EMAIL_HEADER_MUTATOR( # pylint:
disable=invalid-name,unused-argument
WEBDRIVER_OPTION_ARGS = ["--headless"]
# The base URL to query for accessing the user interface
-WEBDRIVER_BASEURL = "http://0.0.0.0:8080/";
+WEBDRIVER_BASEURL = "http://localhost:8088/";
Review Comment:
See my previous comment.
##########
superset/config.py:
##########
@@ -1296,7 +1298,7 @@ def EMAIL_HEADER_MUTATOR( # pylint:
disable=invalid-name,unused-argument
# chrome:
# Requires: headless chrome
# Limitations: unable to generate screenshots of elements
-WEBDRIVER_TYPE = "firefox"
+WEBDRIVER_TYPE = "chrome"
Review Comment:
See my previous comment.
##########
superset/models/dashboard.py:
##########
@@ -160,6 +160,7 @@ class Dashboard(Model, AuditMixinNullable,
ImportExportMixin):
published = Column(Boolean, default=False)
is_managed_externally = Column(Boolean, nullable=False, default=False)
external_url = Column(Text, nullable=True)
+ static_image = Column(Text, nullable=True)
Review Comment:
Shouldn't this be a `BLOB` (or similar) if we're storing a binary object.
Additionally could you talk to any performance concerns this could have, i.e.,
the payload for fetching a record will likely increase significantly.
I'm not sure if having a separate table to house these images makes more
sense.
cc @betodealmeida @michael-s-molina
--
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]
