Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reporting / Alert scheduling doesn't properly convert timezones #29797

Closed
3 tasks done
danielli-ziprecruiter opened this issue Jul 31, 2024 · 2 comments
Closed
3 tasks done
Labels
alert-reports Namespace | Anything related to the Alert & Reports feature global:timezone Related to timezones

Comments

@danielli-ziprecruiter
Copy link
Contributor

Bug description

Operating System: Ubuntu 22.04
Version: 4.0.2

Create an alert that will trigger.

How to reproduce the bug

  1. Create an alert that will trigger.
  2. Schedule the alert to run soon with a timezone other than UTC.
  3. Verify that alert doesn't run when it should.

Screenshots/recordings

No response

Superset version

4.0.2

Python version

3.10

Node version

16

Browser

Chrome

Additional context

No response

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
@dosubot dosubot bot added alert-reports Namespace | Anything related to the Alert & Reports feature global:timezone Related to timezones labels Jul 31, 2024
Copy link

dosubot bot commented Jul 31, 2024

Hey @danielli-ziprecruiter! I'm here to help you with this issue. I'm a bot designed to assist with bug reports, questions, and contributions. Let's tackle this together!

To resolve the issue with the alert scheduling feature in Superset not properly converting timezones, you need to ensure that the triggered_at datetime is correctly converted to the specified timezone and then back to UTC for scheduling. Here is the relevant part of the code that deals with timezone conversion:

def cron_schedule_window(
    triggered_at: datetime, cron: str, timezone: str
) -> Iterator[datetime]:
    window_size = app.config["ALERT_REPORTS_CRON_WINDOW_SIZE"]
    try:
        tz = pytz_timezone(timezone)
    except UnknownTimeZoneError:
        # fallback to default timezone
        tz = pytz_timezone("UTC")
        logger.warning("Timezone %s was invalid. Falling back to 'UTC'", timezone)
    utc = pytz_timezone("UTC")
    # convert the current time to the user's local time for comparison
    time_now = triggered_at.astimezone(tz)
    start_at = time_now - timedelta(seconds=window_size / 2)
    stop_at = time_now + timedelta(seconds=window_size / 2)
    crons = croniter(cron, start_at)
    for schedule in crons.all_next(datetime):
        if schedule >= stop_at:
            break
        # convert schedule back to utc
        yield schedule.astimezone(utc).replace(tzinfo=None)

Ensure that:

  1. The timezone parameter is correctly passed and is a valid timezone string.
  2. The triggered_at datetime is in UTC before being passed to this function.

Additionally, you can refer to the integration tests in tests/integration_tests/reports/api_tests.py to verify the behavior of the alert scheduling feature. For example, the test case for creating a report schedule with an invalid timezone ensures that the system correctly handles invalid timezones:

# Test that report cannot be created with an invalid timezone
report_schedule_data = {
    "type": ReportScheduleType.ALERT,
    "name": "new5",
    "description": "description",
    "creation_method": ReportCreationMethod.ALERTS_REPORTS,
    "crontab": "0 9 * * *",
    "recipients": [
        {
            "type": ReportRecipientType.EMAIL,
            "recipient_config_json": {"target": "target@superset.org"},
        },
        {
            "type": ReportRecipientType.SLACK,
            "recipient_config_json": {"target": "channel"},
        },
    ],
    "working_timeout": 3600,
    "timezone": "this is not a timezone",
    "dashboard": dashboard.id,
    "database": example_db.id,
}
rv = self.post_assert_metric(uri, report_schedule_data, "post")
assert rv.status_code == 400
data = json.loads(rv.data.decode("utf-8"))
assert data == {
    "message": {
        "timezone": [f"Must be one of: {', '.join(pytz.all_timezones)}."]
    }
}

If the issue persists, you might want to add additional logging to verify the values of triggered_at, timezone, and the computed time_now, start_at, and stop_at to debug the issue further [1][2].

Additionally, ensure that the timezones for both the Apache Superset backend and the database engine are configured to use the same timezone, preferably UTC, to avoid inconsistencies. This consistency helps in maintaining accurate scheduling and alerting functionalities [3].

To continue talking to Dosu, mention @dosu.

@rusackas
Copy link
Member

rusackas commented Aug 5, 2024

Fixed by #29798

@rusackas rusackas closed this as completed Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alert-reports Namespace | Anything related to the Alert & Reports feature global:timezone Related to timezones
Projects
None yet
Development

No branches or pull requests

2 participants