[charts] feat: add statsd to charts api (#9571)

* add statsd to charts api

* update test for charts api

* [charts] add statsd asserts wrapper

* [charts] update api test

* removed white space
This commit is contained in:
Lily Kuang 2020-04-21 11:57:42 -07:00 committed by GitHub
parent ba691d3a27
commit 7cefc89c64
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 73 additions and 62 deletions

View File

@ -56,7 +56,11 @@ from superset.models.slice import Slice
from superset.tasks.thumbnails import cache_chart_thumbnail
from superset.utils.core import json_int_dttm_ser
from superset.utils.screenshots import ChartScreenshot
from superset.views.base_api import BaseSupersetModelRestApi, RelatedFieldFilter
from superset.views.base_api import (
BaseSupersetModelRestApi,
RelatedFieldFilter,
statsd_metrics,
)
from superset.views.filters import FilterRelatedOwners
logger = logging.getLogger(__name__)
@ -147,6 +151,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
@expose("/", methods=["POST"])
@protect()
@safe
@statsd_metrics
def post(self) -> Response:
"""Creates a new Chart
---
@ -199,6 +204,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
@expose("/<pk>", methods=["PUT"])
@protect()
@safe
@statsd_metrics
def put( # pylint: disable=too-many-return-statements, arguments-differ
self, pk: int
) -> Response:
@ -266,6 +272,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
@expose("/<pk>", methods=["DELETE"])
@protect()
@safe
@statsd_metrics
def delete(self, pk: int) -> Response: # pylint: disable=arguments-differ
"""Deletes a Chart
---
@ -312,6 +319,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
@expose("/", methods=["DELETE"])
@protect()
@safe
@statsd_metrics
@rison(get_delete_ids_schema)
def bulk_delete(
self, **kwargs: Any
@ -373,6 +381,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
@event_logger.log_this
@protect()
@safe
@statsd_metrics
def data(self) -> Response:
"""
Takes a query context constructed in the client and returns payload
@ -429,6 +438,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
@protect()
@rison(thumbnail_query_schema)
@safe
@statsd_metrics
def thumbnail(
self, pk: int, digest: str, **kwargs: Dict[str, bool]
) -> WerkzeugResponse:

View File

@ -97,20 +97,20 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_delete_chart(self):
"""
Chart API: Test delete
Chart API: Test delete
"""
admin_id = self.get_user("admin").id
chart_id = self.insert_chart("name", [admin_id], 1).id
self.login(username="admin")
uri = f"api/v1/chart/{chart_id}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "delete")
self.assertEqual(rv.status_code, 200)
model = db.session.query(Slice).get(chart_id)
self.assertEqual(model, None)
def test_delete_bulk_charts(self):
"""
Chart API: Test delete bulk
Chart API: Test delete bulk
"""
admin_id = self.get_user("admin").id
chart_count = 4
@ -122,7 +122,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
self.login(username="admin")
argument = chart_ids
uri = f"api/v1/chart/?q={prison.dumps(argument)}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "bulk_delete")
self.assertEqual(rv.status_code, 200)
response = json.loads(rv.data.decode("utf-8"))
expected_response = {"message": f"Deleted {chart_count} charts"}
@ -133,54 +133,54 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_delete_bulk_chart_bad_request(self):
"""
Chart API: Test delete bulk bad request
Chart API: Test delete bulk bad request
"""
chart_ids = [1, "a"]
self.login(username="admin")
argument = chart_ids
uri = f"api/v1/chart/?q={prison.dumps(argument)}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "bulk_delete")
self.assertEqual(rv.status_code, 400)
def test_delete_not_found_chart(self):
"""
Chart API: Test not found delete
Chart API: Test not found delete
"""
self.login(username="admin")
chart_id = 1000
uri = f"api/v1/chart/{chart_id}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "delete")
self.assertEqual(rv.status_code, 404)
def test_delete_bulk_charts_not_found(self):
"""
Chart API: Test delete bulk not found
Chart API: Test delete bulk not found
"""
max_id = db.session.query(func.max(Slice.id)).scalar()
chart_ids = [max_id + 1, max_id + 2]
self.login(username="admin")
argument = chart_ids
uri = f"api/v1/chart/?q={prison.dumps(argument)}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "bulk_delete")
self.assertEqual(rv.status_code, 404)
def test_delete_chart_admin_not_owned(self):
"""
Chart API: Test admin delete not owned
Chart API: Test admin delete not owned
"""
gamma_id = self.get_user("gamma").id
chart_id = self.insert_chart("title", [gamma_id], 1).id
self.login(username="admin")
uri = f"api/v1/chart/{chart_id}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "delete")
self.assertEqual(rv.status_code, 200)
model = db.session.query(Slice).get(chart_id)
self.assertEqual(model, None)
def test_delete_bulk_chart_admin_not_owned(self):
"""
Chart API: Test admin delete bulk not owned
Chart API: Test admin delete bulk not owned
"""
gamma_id = self.get_user("gamma").id
chart_count = 4
@ -193,7 +193,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
self.login(username="admin")
argument = chart_ids
uri = f"api/v1/chart/?q={prison.dumps(argument)}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "bulk_delete")
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(rv.status_code, 200)
expected_response = {"message": f"Deleted {chart_count} charts"}
@ -205,7 +205,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_delete_chart_not_owned(self):
"""
Chart API: Test delete try not owned
Chart API: Test delete try not owned
"""
user_alpha1 = self.create_user(
"alpha1", "password", "Alpha", email="alpha1@superset.org"
@ -216,7 +216,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
chart = self.insert_chart("title", [user_alpha1.id], 1)
self.login(username="alpha2", password="password")
uri = f"api/v1/chart/{chart.id}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "delete")
self.assertEqual(rv.status_code, 403)
db.session.delete(chart)
db.session.delete(user_alpha1)
@ -225,7 +225,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_delete_bulk_chart_not_owned(self):
"""
Chart API: Test delete bulk try not owned
Chart API: Test delete bulk try not owned
"""
user_alpha1 = self.create_user(
"alpha1", "password", "Alpha", email="alpha1@superset.org"
@ -248,7 +248,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
# verify we can't delete not owned charts
arguments = [chart.id for chart in charts]
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "bulk_delete")
self.assertEqual(rv.status_code, 403)
response = json.loads(rv.data.decode("utf-8"))
expected_response = {"message": "Forbidden"}
@ -257,7 +257,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
# # nothing is deleted in bulk with a list of owned and not owned charts
arguments = [chart.id for chart in charts] + [owned_chart.id]
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.client.delete(uri)
rv = self.delete_assert_metric(uri, "bulk_delete")
self.assertEqual(rv.status_code, 403)
response = json.loads(rv.data.decode("utf-8"))
expected_response = {"message": "Forbidden"}
@ -272,7 +272,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_create_chart(self):
"""
Chart API: Test create chart
Chart API: Test create chart
"""
admin_id = self.get_user("admin").id
chart_data = {
@ -288,7 +288,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
}
self.login(username="admin")
uri = f"api/v1/chart/"
rv = self.client.post(uri, json=chart_data)
rv = self.post_assert_metric(uri, chart_data, "post")
self.assertEqual(rv.status_code, 201)
data = json.loads(rv.data.decode("utf-8"))
model = db.session.query(Slice).get(data.get("id"))
@ -297,7 +297,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_create_simple_chart(self):
"""
Chart API: Test create simple chart
Chart API: Test create simple chart
"""
chart_data = {
"slice_name": "title1",
@ -306,7 +306,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
}
self.login(username="admin")
uri = f"api/v1/chart/"
rv = self.client.post(uri, json=chart_data)
rv = self.post_assert_metric(uri, chart_data, "post")
self.assertEqual(rv.status_code, 201)
data = json.loads(rv.data.decode("utf-8"))
model = db.session.query(Slice).get(data.get("id"))
@ -315,7 +315,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_create_chart_validate_owners(self):
"""
Chart API: Test create validate owners
Chart API: Test create validate owners
"""
chart_data = {
"slice_name": "title1",
@ -325,7 +325,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
}
self.login(username="admin")
uri = f"api/v1/chart/"
rv = self.client.post(uri, json=chart_data)
rv = self.post_assert_metric(uri, chart_data, "post")
self.assertEqual(rv.status_code, 422)
response = json.loads(rv.data.decode("utf-8"))
expected_response = {"message": {"owners": ["Owners are invalid"]}}
@ -333,7 +333,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_create_chart_validate_params(self):
"""
Chart API: Test create validate params json
Chart API: Test create validate params json
"""
chart_data = {
"slice_name": "title1",
@ -343,12 +343,12 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
}
self.login(username="admin")
uri = f"api/v1/chart/"
rv = self.client.post(uri, json=chart_data)
rv = self.post_assert_metric(uri, chart_data, "post")
self.assertEqual(rv.status_code, 400)
def test_create_chart_validate_datasource(self):
"""
Chart API: Test create validate datasource
Chart API: Test create validate datasource
"""
self.login(username="admin")
chart_data = {
@ -357,7 +357,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
"datasource_type": "unknown",
}
uri = f"api/v1/chart/"
rv = self.client.post(uri, json=chart_data)
rv = self.post_assert_metric(uri, chart_data, "post")
self.assertEqual(rv.status_code, 422)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(
@ -369,7 +369,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
"datasource_type": "table",
}
uri = f"api/v1/chart/"
rv = self.client.post(uri, json=chart_data)
rv = self.post_assert_metric(uri, chart_data, "post")
self.assertEqual(rv.status_code, 422)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(
@ -378,7 +378,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_update_chart(self):
"""
Chart API: Test update
Chart API: Test update
"""
admin = self.get_user("admin")
gamma = self.get_user("gamma")
@ -397,7 +397,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
}
self.login(username="admin")
uri = f"api/v1/chart/{chart_id}"
rv = self.client.put(uri, json=chart_data)
rv = self.put_assert_metric(uri, chart_data, "put")
self.assertEqual(rv.status_code, 200)
model = db.session.query(Slice).get(chart_id)
related_dashboard = db.session.query(Dashboard).get(1)
@ -417,7 +417,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_update_chart_new_owner(self):
"""
Chart API: Test update set new owner to current user
Chart API: Test update set new owner to current user
"""
gamma = self.get_user("gamma")
admin = self.get_user("admin")
@ -425,7 +425,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
chart_data = {"slice_name": "title1_changed"}
self.login(username="admin")
uri = f"api/v1/chart/{chart_id}"
rv = self.client.put(uri, json=chart_data)
rv = self.put_assert_metric(uri, chart_data, "put")
self.assertEqual(rv.status_code, 200)
model = db.session.query(Slice).get(chart_id)
self.assertIn(admin, model.owners)
@ -434,7 +434,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_update_chart_not_owned(self):
"""
Chart API: Test update not owned
Chart API: Test update not owned
"""
user_alpha1 = self.create_user(
"alpha1", "password", "Alpha", email="alpha1@superset.org"
@ -447,7 +447,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
self.login(username="alpha2", password="password")
chart_data = {"slice_name": "title1_changed"}
uri = f"api/v1/chart/{chart.id}"
rv = self.client.put(uri, json=chart_data)
rv = self.put_assert_metric(uri, chart_data, "put")
self.assertEqual(rv.status_code, 403)
db.session.delete(chart)
db.session.delete(user_alpha1)
@ -456,14 +456,14 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_update_chart_validate_datasource(self):
"""
Chart API: Test update validate datasource
Chart API: Test update validate datasource
"""
admin = self.get_user("admin")
chart = self.insert_chart("title", [admin.id], 1)
self.login(username="admin")
chart_data = {"datasource_id": 1, "datasource_type": "unknown"}
uri = f"api/v1/chart/{chart.id}"
rv = self.client.put(uri, json=chart_data)
rv = self.put_assert_metric(uri, chart_data, "put")
self.assertEqual(rv.status_code, 422)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(
@ -471,7 +471,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
)
chart_data = {"datasource_id": 0, "datasource_type": "table"}
uri = f"api/v1/chart/{chart.id}"
rv = self.client.put(uri, json=chart_data)
rv = self.put_assert_metric(uri, chart_data, "put")
self.assertEqual(rv.status_code, 422)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(
@ -482,7 +482,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_update_chart_validate_owners(self):
"""
Chart API: Test update validate owners
Chart API: Test update validate owners
"""
chart_data = {
"slice_name": "title1",
@ -500,13 +500,13 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_get_chart(self):
"""
Chart API: Test get chart
Chart API: Test get chart
"""
admin = self.get_user("admin")
chart = self.insert_chart("title", [admin.id], 1)
self.login(username="admin")
uri = f"api/v1/chart/{chart.id}"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get")
self.assertEqual(rv.status_code, 200)
expected_result = {
"cache_timeout": None,
@ -531,17 +531,17 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_get_chart_not_found(self):
"""
Chart API: Test get chart not found
Chart API: Test get chart not found
"""
chart_id = 1000
self.login(username="admin")
uri = f"api/v1/chart/{chart_id}"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get")
self.assertEqual(rv.status_code, 404)
def test_get_chart_no_data_access(self):
"""
Chart API: Test get chart without data access
Chart API: Test get chart without data access
"""
self.login(username="gamma")
chart_no_access = (
@ -555,30 +555,30 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_get_charts(self):
"""
Chart API: Test get charts
Chart API: Test get charts
"""
self.login(username="admin")
uri = f"api/v1/chart/"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 33)
def test_get_charts_filter(self):
"""
Chart API: Test get charts filter
Chart API: Test get charts filter
"""
self.login(username="admin")
arguments = {"filters": [{"col": "slice_name", "opr": "sw", "value": "G"}]}
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 5)
def test_get_charts_custom_filter(self):
"""
Chart API: Test get charts custom filter
Chart API: Test get charts custom filter
"""
admin = self.get_user("admin")
chart1 = self.insert_chart("foo", [admin.id], 1, description="ZY_bar")
@ -595,7 +595,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
}
self.login(username="admin")
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 3)
@ -614,7 +614,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
self.logout()
self.login(username="gamma")
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 0)
@ -628,7 +628,7 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_get_charts_page(self):
"""
Chart API: Test get charts filter
Chart API: Test get charts filter
"""
# Assuming we have 33 sample charts
self.login(username="admin")
@ -641,36 +641,37 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
arguments = {"page_size": 10, "page": 3}
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(len(data["result"]), 3)
def test_get_charts_no_data_access(self):
"""
Chart API: Test get charts no data access
Chart API: Test get charts no data access
"""
self.login(username="gamma")
uri = f"api/v1/chart/"
rv = self.client.get(uri)
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 0)
def test_chart_data(self):
"""
Query API: Test chart data query
Query API: Test chart data query
"""
self.login(username="admin")
query_context = self._get_query_context()
uri = "api/v1/chart/data"
rv = self.client.post(uri, json=query_context)
rv = self.post_assert_metric(uri, query_context, "data")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["result"][0]["rowcount"], 100)
def test_invalid_chart_data(self):
"""Query API: Test chart data query with invalid schema
"""
Query API: Test chart data query with invalid schema
"""
self.login(username="admin")
query_context = self._get_query_context()
@ -681,10 +682,10 @@ class ChartApiTests(SupersetTestCase, ApiOwnersTestCaseMixin):
def test_query_exec_not_allowed(self):
"""
Query API: Test chart data query not allowed
Query API: Test chart data query not allowed
"""
self.login(username="gamma")
query_context = self._get_query_context()
uri = "api/v1/chart/data"
rv = self.client.post(uri, json=query_context)
rv = self.post_assert_metric(uri, query_context, "data")
self.assertEqual(rv.status_code, 401)