You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2021/11/22 19:54:57 UTC
[superset] branch 1.4 updated (8a24374 -> 7d9f63e)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a change to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git.
from 8a24374 fix(AlertReportModal): Text Area Change (#17176)
new 680cdca fix: update values for default timezone selector (#17124)
new 30c6ec0 fix: Allow users to update database in Dataset Edit Modal (#17265)
new 7d9f63e fix: add fallback and validation for report and cron timezones (#17338)
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CONTRIBUTING.md | 13 ++++++--
.../TimezoneSelector/TimezoneSelector.test.tsx | 29 +++++++++++++++--
.../src/components/TimezoneSelector/index.tsx | 33 +++++++++++--------
.../src/datasource/DatasourceEditor.jsx | 2 +-
superset/reports/schemas.py | 13 ++++++--
superset/tasks/cron_util.py | 14 ++++++--
tests/integration_tests/reports/api_tests.py | 33 +++++++++++++++++++
tests/unit_tests/tasks/test_cron_util.py | 38 ++++++++++++++++++++++
8 files changed, 151 insertions(+), 24 deletions(-)
[superset] 03/03: fix: add fallback and validation for report and cron timezones (#17338)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7d9f63eda71635681c9592f6ed739ccc37d30966
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri Nov 12 12:28:17 2021 -0800
fix: add fallback and validation for report and cron timezones (#17338)
* add fallback and validation for report and cron timezones
* add logging to exception catch
* Run black
Co-authored-by: Beto Dealmeida <ro...@dealmeida.net>
(cherry picked from commit f10bc6d8fe7f3fa4056db2aaff8256f9c3e1550b)
---
CONTRIBUTING.md | 13 +++++++---
superset/reports/schemas.py | 13 ++++++++--
superset/tasks/cron_util.py | 14 +++++++---
tests/integration_tests/reports/api_tests.py | 33 ++++++++++++++++++++++++
tests/unit_tests/tasks/test_cron_util.py | 38 ++++++++++++++++++++++++++++
5 files changed, 103 insertions(+), 8 deletions(-)
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 134cdef..8a82a8d 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -420,7 +420,7 @@ For example, the image referenced above actually lives in `superset-frontend/src
#### OS Dependencies
-Make sure your machine meets the [OS dependencies](https://superset.apache.org/docs/installation/installing-superset-from-scratch#os-dependencies) before following these steps.
+Make sure your machine meets the [OS dependencies](https://superset.apache.org/docs/installation/installing-superset-from-scratch#os-dependencies) before following these steps.
You also need to install MySQL or [MariaDB](https://mariadb.com/downloads).
Ensure that you are using Python version 3.7 or 3.8, then proceed with:
@@ -523,6 +523,7 @@ Frontend assets (TypeScript, JavaScript, CSS, and images) must be compiled in or
##### nvm and node
First, be sure you are using the following versions of Node.js and npm:
+
- `Node.js`: Version 16
- `npm`: Version 7
@@ -752,15 +753,21 @@ Note that the test environment uses a temporary directory for defining the
SQLite databases which will be cleared each time before the group of test
commands are invoked.
-There is also a utility script included in the Superset codebase to run python tests. The [readme can be
+There is also a utility script included in the Superset codebase to run python integration tests. The [readme can be
found here](https://github.com/apache/superset/tree/master/scripts/tests)
-To run all tests for example, run this script from the root directory:
+To run all integration tests for example, run this script from the root directory:
```bash
scripts/tests/run.sh
```
+You can run unit tests found in './tests/unit_tests' for example with pytest. It is a simple way to run an isolated test that doesn't need any database setup
+
+```bash
+pytest ./link_to_test.py
+```
+
### Frontend Testing
We use [Jest](https://jestjs.io/) and [Enzyme](https://airbnb.io/enzyme/) to test TypeScript/JavaScript. Tests can be run with:
diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py
index 2180e4d..c8486b8 100644
--- a/superset/reports/schemas.py
+++ b/superset/reports/schemas.py
@@ -21,6 +21,7 @@ from flask_babel import gettext as _
from marshmallow import fields, Schema, validate, validates_schema
from marshmallow.validate import Length, Range, ValidationError
from marshmallow_enum import EnumField
+from pytz import all_timezones
from superset.models.reports import (
ReportCreationMethodType,
@@ -153,7 +154,11 @@ class ReportSchedulePostSchema(Schema):
allow_none=False,
required=True,
)
- timezone = fields.String(description=timezone_description, default="UTC")
+ timezone = fields.String(
+ description=timezone_description,
+ default="UTC",
+ validate=validate.OneOf(choices=tuple(all_timezones)),
+ )
sql = fields.String(
description=sql_description, example="SELECT value FROM time_series_table"
)
@@ -233,7 +238,11 @@ class ReportSchedulePutSchema(Schema):
validate=[validate_crontab, Length(1, 1000)],
required=False,
)
- timezone = fields.String(description=timezone_description, default="UTC")
+ timezone = fields.String(
+ description=timezone_description,
+ default="UTC",
+ validate=validate.OneOf(choices=tuple(all_timezones)),
+ )
sql = fields.String(
description=sql_description,
example="SELECT value FROM time_series_table",
diff --git a/superset/tasks/cron_util.py b/superset/tasks/cron_util.py
index 3824382..9c275ad 100644
--- a/superset/tasks/cron_util.py
+++ b/superset/tasks/cron_util.py
@@ -15,21 +15,29 @@
# specific language governing permissions and limitations
# under the License.
+import logging
from datetime import datetime, timedelta, timezone as dt_timezone
from typing import Iterator
-import pytz
from croniter import croniter
+from pytz import timezone as pytz_timezone, UnknownTimeZoneError
from superset import app
+logger = logging.getLogger(__name__)
+
def cron_schedule_window(cron: str, timezone: str) -> Iterator[datetime]:
window_size = app.config["ALERT_REPORTS_CRON_WINDOW_SIZE"]
# create a time-aware datetime in utc
time_now = datetime.now(tz=dt_timezone.utc)
- tz = pytz.timezone(timezone)
- utc = pytz.timezone("UTC")
+ 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 = time_now.astimezone(tz)
start_at = time_now - timedelta(seconds=1)
diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py
index 7526d11..8df8b4b 100644
--- a/tests/integration_tests/reports/api_tests.py
+++ b/tests/integration_tests/reports/api_tests.py
@@ -19,6 +19,8 @@
from datetime import datetime
import json
+import pytz
+
import pytest
import prison
from sqlalchemy.sql import func
@@ -706,6 +708,37 @@ class TestReportSchedulesApi(SupersetTestCase):
data = json.loads(rv.data.decode("utf-8"))
assert data == {"message": {"timezone": ["Field may not be null."]}}
+ # Test that report cannot be created with an invalid timezone
+ report_schedule_data = {
+ "type": ReportScheduleType.ALERT,
+ "name": "new5",
+ "description": "description",
+ "creation_method": ReportCreationMethodType.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.client.post(uri, json=report_schedule_data)
+ 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)}."]
+ }
+ }
+
# Test that report should reflect the timezone value passed in
report_schedule_data = {
"type": ReportScheduleType.ALERT,
diff --git a/tests/unit_tests/tasks/test_cron_util.py b/tests/unit_tests/tasks/test_cron_util.py
index c905df2..9042cca 100644
--- a/tests/unit_tests/tasks/test_cron_util.py
+++ b/tests/unit_tests/tasks/test_cron_util.py
@@ -67,6 +67,44 @@ def test_cron_schedule_window_los_angeles(
@pytest.mark.parametrize(
"current_dttm, cron, expected",
[
+ ("2020-01-01T00:59:01Z", "0 1 * * *", []),
+ (
+ "2020-01-01T00:59:02Z",
+ "0 1 * * *",
+ [FakeDatetime(2020, 1, 1, 1, 0).strftime("%A, %d %B %Y, %H:%M:%S")],
+ ),
+ (
+ "2020-01-01T00:59:59Z",
+ "0 1 * * *",
+ [FakeDatetime(2020, 1, 1, 1, 0).strftime("%A, %d %B %Y, %H:%M:%S")],
+ ),
+ (
+ "2020-01-01T01:00:00Z",
+ "0 1 * * *",
+ [FakeDatetime(2020, 1, 1, 1, 0).strftime("%A, %d %B %Y, %H:%M:%S")],
+ ),
+ ("2020-01-01T01:00:01Z", "0 1 * * *", []),
+ ],
+)
+def test_cron_schedule_window_invalid_timezone(
+ app_context: AppContext, current_dttm: str, cron: str, expected: List[FakeDatetime]
+) -> None:
+ """
+ Reports scheduler: Test cron schedule window for "invalid timezone"
+ """
+
+ with freeze_time(current_dttm):
+ datetimes = cron_schedule_window(cron, "invalid timezone")
+ # it should default to UTC
+ assert (
+ list(cron.strftime("%A, %d %B %Y, %H:%M:%S") for cron in datetimes)
+ == expected
+ )
+
+
+@pytest.mark.parametrize(
+ "current_dttm, cron, expected",
+ [
("2020-01-01T05:59:01Z", "0 1 * * *", []),
(
"2020-01-01T05:59:02Z",
[superset] 02/03: fix: Allow users to update database in Dataset Edit Modal (#17265)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 30c6ec07ef8f9b4601981cabb2e74dc6d0131b2a
Author: Hugh A. Miles II <hu...@gmail.com>
AuthorDate: Fri Oct 29 17:19:37 2021 -0400
fix: Allow users to update database in Dataset Edit Modal (#17265)
* add condition to fix save
* remove console.log
(cherry picked from commit d0bad96b1ab9065a5e1d313793da35089ee1f07c)
---
superset-frontend/src/datasource/DatasourceEditor.jsx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx
index e80e402..8150971 100644
--- a/superset-frontend/src/datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/datasource/DatasourceEditor.jsx
@@ -471,7 +471,6 @@ class DatasourceEditor extends React.PureComponent {
const { datasourceType, datasource } = this.state;
const sql =
datasourceType === DATASOURCE_TYPES.physical.key ? '' : datasource.sql;
-
const newDatasource = {
...this.state.datasource,
sql,
@@ -489,6 +488,7 @@ class DatasourceEditor extends React.PureComponent {
}
onDatasourcePropChange(attr, value) {
+ if (value === undefined) return; // if value is undefined do not update state
const datasource = { ...this.state.datasource, [attr]: value };
this.setState(
prevState => ({
[superset] 01/03: fix: update values for default timezone selector (#17124)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 680cdca14a9b4d5c3e271ec723b074951273dd06
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Mon Oct 18 09:39:20 2021 -0700
fix: update values for default timezone selector (#17124)
* update values for default timezone selector
* fix casing and comment
* Update TimezoneSelector.test.tsx
(cherry picked from commit ae4ced8da6933cdc657452d1f11415c49c6c68b8)
---
.../TimezoneSelector/TimezoneSelector.test.tsx | 29 +++++++++++++++++--
.../src/components/TimezoneSelector/index.tsx | 33 +++++++++++++---------
2 files changed, 47 insertions(+), 15 deletions(-)
diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx
index f0b12d4..faa38b0 100644
--- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx
+++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx
@@ -18,14 +18,18 @@
*/
import React from 'react';
import moment from 'moment-timezone';
-import { render } from 'spec/helpers/testing-library';
+import { render, screen } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
import TimezoneSelector from './index';
describe('TimezoneSelector', () => {
- let timezone: string;
+ let timezone: string | undefined;
const onTimezoneChange = jest.fn(zone => {
timezone = zone;
});
+ beforeEach(() => {
+ timezone = undefined;
+ });
it('renders a TimezoneSelector with a default if undefined', () => {
jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York');
render(
@@ -36,6 +40,27 @@ describe('TimezoneSelector', () => {
);
expect(onTimezoneChange).toHaveBeenCalledWith('America/Nassau');
});
+ it('should properly select values from the offsetsToName map', async () => {
+ jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York');
+ render(
+ <TimezoneSelector
+ onTimezoneChange={onTimezoneChange}
+ timezone={timezone}
+ />,
+ );
+
+ const select = screen.getByRole('combobox', {
+ name: 'Timezone selector',
+ });
+ expect(select).toBeInTheDocument();
+ userEvent.click(select);
+ const selection = await screen.findByTitle(
+ 'GMT -06:00 (Mountain Daylight Time)',
+ );
+ expect(selection).toBeInTheDocument();
+ userEvent.click(selection);
+ expect(selection).toBeVisible();
+ });
it('renders a TimezoneSelector with the closest value if passed in', async () => {
render(
<TimezoneSelector
diff --git a/superset-frontend/src/components/TimezoneSelector/index.tsx b/superset-frontend/src/components/TimezoneSelector/index.tsx
index 48dd100..a028991 100644
--- a/superset-frontend/src/components/TimezoneSelector/index.tsx
+++ b/superset-frontend/src/components/TimezoneSelector/index.tsx
@@ -17,12 +17,16 @@
* under the License.
*/
-import React, { useEffect, useRef } from 'react';
+import React, { useEffect, useRef, useCallback } from 'react';
import moment from 'moment-timezone';
import { t } from '@superset-ui/core';
import { Select } from 'src/components';
-const DEFAULT_TIMEZONE = 'GMT Standard Time';
+const DEFAULT_TIMEZONE = {
+ name: 'GMT Standard Time',
+ value: 'Africa/Abidjan', // timezones are deduped by the first alphabetical value
+};
+
const MIN_SELECT_WIDTH = '400px';
const offsetsToName = {
@@ -37,7 +41,7 @@ const offsetsToName = {
'-540-480': ['Alaska Standard Time', 'Alaska Daylight Time'],
'-600-600': ['Hawaii Standard Time', 'Hawaii Daylight Time'],
'60120': ['Central European Time', 'Central European Daylight Time'],
- '00': [DEFAULT_TIMEZONE, DEFAULT_TIMEZONE],
+ '00': [DEFAULT_TIMEZONE.name, DEFAULT_TIMEZONE.name],
'060': ['GMT Standard Time - London', 'British Summer Time'],
};
@@ -96,28 +100,31 @@ const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => {
const prevTimezone = useRef(timezone);
const matchTimezoneToOptions = (timezone: string) =>
TIMEZONE_OPTIONS.find(option => option.offsets === getOffsetKey(timezone))
- ?.value || DEFAULT_TIMEZONE;
+ ?.value || DEFAULT_TIMEZONE.value;
- const updateTimezone = (tz: string) => {
- // update the ref to track changes
- prevTimezone.current = tz;
- // the parent component contains the state for the value
- onTimezoneChange(tz);
- };
+ const updateTimezone = useCallback(
+ (tz: string) => {
+ // update the ref to track changes
+ prevTimezone.current = tz;
+ // the parent component contains the state for the value
+ onTimezoneChange(tz);
+ },
+ [onTimezoneChange],
+ );
useEffect(() => {
const updatedTz = matchTimezoneToOptions(timezone || moment.tz.guess());
if (prevTimezone.current !== updatedTz) {
updateTimezone(updatedTz);
}
- }, [timezone]);
+ }, [timezone, updateTimezone]);
return (
<Select
- ariaLabel={t('Timezone')}
+ ariaLabel={t('Timezone selector')}
css={{ minWidth: MIN_SELECT_WIDTH }} // smallest size for current values
onChange={onTimezoneChange}
- value={timezone || DEFAULT_TIMEZONE}
+ value={timezone || DEFAULT_TIMEZONE.value}
options={TIMEZONE_OPTIONS}
/>
);