You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "villebro (via GitHub)" <gi...@apache.org> on 2025/01/14 20:54:37 UTC
[PR] chore: replace selenium user with fixed user [superset]
villebro opened a new pull request, #31844:
URL: https://github.com/apache/superset/pull/31844
### SUMMARY
WIP
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: closes #31843
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1924156311
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
@michael-s-molina I kinda like this idea. However, I'm not fully clear on what the exact pattern will look like for the forthcoming `AsyncTaskType`. Similarly, a while back when I was implementing a custom `KeyValue` resource for an internal use case, I ran into issues where it was difficult to extend the existing `KeyValueResource` type. In the end I ended up doing a bunch of funky casting to avoid linting issues. So to get around this, we would need to make sure the type (or its equivalent) supports introducing new types beyond the built-in ones.
Thoughts? I'm ok implementing an `AsyncTaskType` enum (or similar) now, but we may still need to refactor it later, causing yet another breaking change.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1924224271
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
I'd prefer something that serializes more easily. String based enums are good in this regard, but introduce the pluggability issue outlined above.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #31844:
URL: https://github.com/apache/superset/pull/31844#issuecomment-2591092329
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/31844?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `83.33333%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 65.52%. Comparing base [(`76d897e`)](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`58127fa`)](https://app.codecov.io/gh/apache/superset/commit/58127fa61f5947ee4b9fccb46f2f242e667c283a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1317 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/31844?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/tasks/cache.py](https://app.codecov.io/gh/apache/superset/pull/31844?src=pr&el=tree&filepath=superset%2Ftasks%2Fcache.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdGFza3MvY2FjaGUucHk=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/31844?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #31844 +/- ##
==========================================
+ Coverage 60.48% 65.52% +5.03%
==========================================
Files 1931 539 -1392
Lines 76236 39057 -37179
Branches 8568 0 -8568
==========================================
- Hits 46114 25593 -20521
+ Misses 28017 13464 -14553
+ Partials 2105 0 -2105
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/31844/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/31844/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.84% <50.00%> (-0.33%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/31844/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/31844/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.36% <50.00%> (-0.45%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/31844/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.52% <83.33%> (+2.03%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/31844/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.05% <83.33%> (+3.42%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/31844?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1925184818
##########
superset/thumbnails/digest.py:
##########
@@ -92,11 +93,15 @@ def _adjust_string_with_rls(
def get_dashboard_digest(dashboard: Dashboard) -> str:
config = current_app.config
datasources = dashboard.datasources
- executor_type, executor = get_executor(
- executor_types=config["THUMBNAIL_EXECUTE_AS"],
- model=dashboard,
- current_user=get_current_user(),
- )
+ try:
+ executor_type, executor = get_executor(
+ executors=config["THUMBNAIL_EXECUTORS"],
+ model=dashboard,
+ current_user=get_current_user(),
+ )
+ except ExecutorNotFoundError:
+ return ""
Review Comment:
Maybe `None` will be a better representation? If you agree, please change all digest generation methods.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1924246474
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
Would a class, with predefined values that can be extended, be serializable enough? Something like:
```
class AsyncTaskType:
CACHE_WARMUP = "CACHE_WARMUP",
ALERTS_REPORTS = "ALERTS_REPORTS",
_additional_types = set()
@classmethod
def add_type(cls, type):
if type not in cls._additional_types:
cls._additional_types.add(type)
@classmethod
def get_types(cls):
return {cls.CACHE_WARMUP, cls.ALERTS_REPORTS}.union(cls._additional_types)
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1925764381
##########
superset/charts/schemas.py:
##########
@@ -162,6 +162,7 @@ class ChartEntityResponseSchema(Schema):
certification_details = fields.String(
metadata={"description": certification_details_description}
)
+ thumbnail_url = fields.String(allow_none=True)
Review Comment:
I noticed that `thumbnail_url` was missing on the Marshmallow schema despite being returned by the API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1924276111
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
This would essentially mean `get_types` just returns `set[str]`, which means `ASYNC_TASKS_EXECUTORS` would be `dict[str, list[ExecutorType]]`. I'm sure there's some good pattern for solving this, but I still feel that's best taken care of in a follow-up PR that implements the new async task framework.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1915591042
##########
superset/config.py:
##########
@@ -689,6 +689,10 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# The username to execute asynchronous tasks as (cache warmup, alerts & reports,
+# thumbnails) if `ExecutorType.FIXED_USER` is chosen as the executor
+ASYNC_TASK_FIXED_USER: str | None = "admin"
Review Comment:
### Insecure default admin user for async tasks <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Default admin user is hardcoded for asynchronous tasks execution without proper security consideration.
###### Why this matters
Using a hardcoded admin user for asynchronous tasks can lead to security vulnerabilities and lack of audit trails as all tasks would be executed with admin privileges by default.
###### Suggested change ∙ *Feature Preview*
Change default to None to force explicit configuration:
```python
ASYNC_TASK_FIXED_USER: str | None = None
```
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
<!--- korbi internal id:c0922580-af6c-435a-b4e0-5e0b045d5d0d -->
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro merged PR #31844:
URL: https://github.com/apache/superset/pull/31844
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #31844:
URL: https://github.com/apache/superset/pull/31844#issuecomment-2605817980
@michael-s-molina is there anything else that needs to be addressed, or is this good to go, assuming the async task type change can wait until the GATF feature?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1924246474
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
Would a class with predefined values that can be extended be serializable enough? Something like:
```
class AsyncTaskType:
CACHE_WARMUP = "CACHE_WARMUP",
ALERTS_REPORTS = "ALERTS_REPORTS",
_additional_types = set()
@classmethod
def add_type(cls, type):
if type not in cls._additional_types:
cls._additional_types.add(type)
@classmethod
def get_types(cls):
return {cls.CACHE_WARMUP, cls.ALERTS_REPORTS}.union(cls._additional_types)
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1922263982
##########
UPDATING.md:
##########
@@ -24,6 +24,7 @@ assists people when migrating to a new version.
## Next
+- [31844](https://github.com/apache/superset/pull/31844) The `ALERT_REPORTS_EXECUTE_AS` and `THUMBNAILS_EXECUTE_AS` config parameters have been renamed to `ALERT_REPORTS_EXECUTORS` and `THUMBNAILS_EXECUTORS` respectively. A new config flag `CACHE_WARMUP_EXECUTORS` has also been introduced to be able to control which user is used to execute cache warmup tasks. Finally addition, the config flag `THUMBNAILS_SELENIUM_USER` has been removed. To use a fixed executor for async tasks, use the new `FixedExecutor` class. See the config
Review Comment:
> See the config
Did you forget to complete the sentence?
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
Would be possible to have a more generic configuration that allows future async tasks and reduces the number of configurations? Something like:
```
ASYNC_TASK_EXECUTORS: dict[AsyncTaskType, ExecutorType] = {}
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1922275787
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
Would be possible to have a more generic configuration that allows future async tasks and reduces the number of configurations? Something like:
```
ASYNC_TASKS_EXECUTORS: dict[AsyncTaskType, ExecutorType] = {}
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1924219026
##########
superset/config.py:
##########
@@ -689,17 +689,29 @@ class D3TimeFormat(TypedDict, total=False):
# This is merely a default
EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = []
+# User used to execute cache warmup tasks
+# By default, the cache is warmed up using the primary owner. To fall back to using
+# a fixed user (admin in this example), use the following configuration:
+#
+# from superset.tasks.types import ExecutorType, FixedExecutor
+#
+# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")]
+CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER]
Review Comment:
How about:
```
class AsyncTaskType:
"""Base class for async task types."""
pass
class CacheWarmUpTask(AsyncTaskType):
pass
class AlertsReportsTask(AsyncTaskType):
pass
# Usage
ASYNC_TASKS_EXECUTORS: dict[AsyncTaskType, ExecutorType] = {
CacheWarmUpTask(): ExecutorType.OWNER,
AlertsReportsTask(): FixedExecutor("admin"),
}
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: replace selenium user with fixed user [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #31844:
URL: https://github.com/apache/superset/pull/31844#discussion_r1925766085
##########
superset/thumbnails/digest.py:
##########
@@ -92,11 +93,15 @@ def _adjust_string_with_rls(
def get_dashboard_digest(dashboard: Dashboard) -> str:
config = current_app.config
datasources = dashboard.datasources
- executor_type, executor = get_executor(
- executor_types=config["THUMBNAIL_EXECUTE_AS"],
- model=dashboard,
- current_user=get_current_user(),
- )
+ try:
+ executor_type, executor = get_executor(
+ executors=config["THUMBNAIL_EXECUTORS"],
+ model=dashboard,
+ current_user=get_current_user(),
+ )
+ except ExecutorNotFoundError:
+ return ""
Review Comment:
I agree. I've updated throughout the codebase (I did the same for `thumbnail_url`, as that should really also be `None` if it's not defined).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org