Vitor-Avila commented on code in PR #44992:
URL: https://github.com/apache/superset/pull/44992#discussion_r4202685386
##########
superset/commands/report/base.py:
##########
@@ -345,45 +393,118 @@ def validate_report_frequency(
report_type: str,
) -> None:
"""
- Validates if the report scheduled frequency doesn't exceed a limit
- configured in `config.py`.
+ Validates if the report scheduled frequency doesn't exceed the minimum
+ interval in effect.
:param cron_schedule: The cron schedule configured.
:param report_type: The report type (Alert/Report).
"""
config_key = (
- "ALERT_MINIMUM_INTERVAL"
+ ReportConfigKey.ALERT_MINIMUM_INTERVAL
if report_type == ReportScheduleType.ALERT
- else "REPORT_MINIMUM_INTERVAL"
+ else ReportConfigKey.REPORT_MINIMUM_INTERVAL
)
- minimum_interval = app.config.get(config_key, 0)
- if callable(minimum_interval):
- minimum_interval = minimum_interval()
+ minimum_interval = ReportConfigDAO.get_effective_value(config_key)
+ if minimum_interval is None:
+ return
if not isinstance(minimum_interval, int):
logger.error(
"Invalid value for %s: %s", config_key, minimum_interval,
exc_info=True
)
return
- # Since configuration is in minutes, we only need to validate
- # in case `minimum_interval` is <= 120 (2min)
- if minimum_interval < 120:
- return
-
- iterations = 60 if minimum_interval <= 3660 else 24
try:
- schedule = croniter(cron_schedule)
- current_exec = next(schedule)
-
- for _i in range(iterations):
- next_exec = next(schedule)
- diff, current_exec = next_exec - current_exec, next_exec
- if int(diff) < minimum_interval:
- raise ReportScheduleFrequencyNotAllowed(
- report_type=report_type,
minimum_interval=minimum_interval
- )
+ if not cron_meets_minimum_interval(cron_schedule,
minimum_interval):
+ raise ReportScheduleFrequencyNotAllowed(
+ report_type=report_type, minimum_interval=minimum_interval
+ )
except CroniterBadDateError as ex:
raise ReportScheduleCrontabNotValidError(
cron_schedule=cron_schedule
) from ex
+
+ def validate_report_format(
+ self,
+ report_type: str,
+ report_format: str | None,
+ exceptions: list[ValidationError],
+ ) -> None:
+ """
+ Reports always deliver content: the "no attachment" format is only
+ allowed on alerts.
+ """
+ if (
+ report_type == ReportScheduleType.REPORT
+ and report_format == ReportDataFormat.NONE
+ ):
+ exceptions.append(ReportScheduleFormatRequiredError())
+
+ def validate_recipients_policy(self, exceptions: list[ValidationError]) ->
None:
+ """
+ Validate the e-mail recipients in the payload against the global
+ recipient policy (allowed domains and/or existing users only).
+ """
+ recipients = self._properties.get("recipients")
+ if not recipients:
+ return
+ disallowed = ReportConfigDAO.find_disallowed_addresses(
+ get_email_addresses(recipients)
+ )
+ if disallowed:
+
exceptions.append(ReportScheduleRecipientNotAllowedError(disallowed))
+
+ def validate_run_as(
+ self,
+ field_name: str,
+ user_id: int | None,
+ exceptions: list[ValidationError],
+ *,
+ default_to_current_user: bool,
+ ) -> User | None:
+ """Validate a specific user or an admin-selected application default.
+
+ A blank query executor inherits the content executor. A blank content
+ executor selected by an admin uses the legacy application
configuration.
+ """
+ is_admin = security_manager.is_admin()
+ current_user = get_user()
+ type_field = f"{field_name}_type"
+ executor_type = self._properties.pop(type_field, None)
+ if executor_type is not None and executor_type !=
ExecutorType.FIXED_USER:
+ if not is_admin:
+
exceptions.append(ReportScheduleRunAsForbiddenError(field_name))
+ return None
+ exceptions.append(
+ ValidationError(
+ _("Choose Application default or a specific user"),
+ field_name=field_name,
+ )
+ )
+ return None
+
+ if user_id is None and executor_type != ExecutorType.FIXED_USER:
+ if field_name == "run_alert_query_as":
Review Comment:
Another great catch! Fixing it
--
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]
