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

docs: improve API v1 migration documentation #23298

Merged
merged 3 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ assists people when migrating to a new version.

## Next

- [22809](https://github.com/apache/superset/pull/22809): Migrated endpoint `/superset/sql_json` and `/superset/results/` to `/api/v1/sqllab/execute/` and `/api/v1/sqllab/results/` respectively. Corresponding permissions are `can sql_json on Superset` to `can execute on SQLLab`, `can results on Superset` to `can results on SQLLab`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22931](https://github.com/apache/superset/pull/22931): Migrated endpoint `/superset/get_or_create_table/` to `/api/v1/dataset/get_or_create/`. Corresponding permissions are `can get or create table on Superset` to `can get or create dataset on Dataset`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22882](https://github.com/apache/superset/pull/22882): Migrated endpoint `/superset/filter/<datasource_type>/<int:datasource_id>/<column>/` to `/api/v1/datasource/<datasource_type>/<datasource_id>/column/<column_name>/values/`. Corresponding permissions are `can filter on Superset` to `can get column values on Datasource`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22789](https://github.com/apache/superset/pull/22789): Migrated endpoint `/superset/recent_activity/<user_id>/` to `/api/v1/log/recent_activity/<user_id>/`. Corresponding permissions are `can recent activity on Superset` to `can recent activity on Log`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22913](https://github.com/apache/superset/pull/22913): Migrated endpoint `/superset/csv` to `/api/v1/sqllab/export/`. Corresponding permissions are `can csv on Superset` to `can export csv on SQLLab`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22496](https://github.com/apache/superset/pull/22496): Migrated endpoint `/superset/slice_json/<int:layer_id>` to `/api/v1/chart/data`. Corresponding permissions are `can slice json on Superset` to `can read on Chart`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22496](https://github.com/apache/superset/pull/22496): Migrated endpoint `/superset/annotation_json/<int:layer_id>` to `/api/v1/chart/data`. Corresponding permissions are `can annotation json on Superset` to `can read on Chart`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22624](https://github.com/apache/superset/pull/22624): Migrated endpoint `/superset/stop_query/` to `/api/v1/query/stop`. Corresponding permissions are `can stop query on Superset` to `can read on Query`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22579](https://github.com/apache/superset/pull/22579): Migrated endpoint `/superset/search_queries/` to `/api/v1/query/`. Corresponding permissions are `can search queries on Superset` to `can read on Query`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22501](https://github.com/apache/superset/pull/22501): Migrated endpoint `/superset/tables/<int:db_id>/<schema>/` to `/api/v1/database/<int:id>/tables/`. Corresponding permissions are `can tables on Superset` to `can read on Database`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [22611](https://github.com/apache/superset/pull/22611): Migrated endpoint `/superset/queries/` to `api/v1/query/updated_since`. Corresponding permissions are `can queries on Superset` to `can read on Query`. Make sure you add/replace the necessary permissions on any custom roles you may have.
- [23186](https://github.com/apache/superset/pull/23186): Superset will refuse to start if a default `SECRET_KEY` is detected on a non Flask debug setting.
- [22022](https://github.com/apache/superset/pull/22022): HTTP API endpoints `/superset/approve` and `/superset/request_access` have been deprecated and their HTTP methods were changed from GET to POST
- [20606](https://github.com/apache/superset/pull/20606): When user clicks on chart title or "Edit chart" button in Dashboard page, Explore opens in the same tab. Clicking while holding cmd/ctrl opens Explore in a new tab. To bring back the old behaviour (always opening Explore in a new tab), flip feature flag `DASHBOARD_EDIT_CHART_IN_NEW_TAB` to `True`.
Expand Down
13 changes: 10 additions & 3 deletions superset/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ def generate_download_headers(

def deprecated(
eol_version: str = "3.0.0",
new_target: Optional[str] = None,
) -> Callable[[Callable[..., FlaskResponse]], Callable[..., FlaskResponse]]:
"""
A decorator to set an API endpoint from SupersetView has deprecated.
Expand All @@ -196,13 +197,19 @@ def deprecated(

def _deprecated(f: Callable[..., FlaskResponse]) -> Callable[..., FlaskResponse]:
def wraps(self: "BaseSupersetView", *args: Any, **kwargs: Any) -> FlaskResponse:
logger.warning(
messsage = (
"%s.%s "
"This API endpoint is deprecated and will be removed in version %s",
"This API endpoint is deprecated and will be removed in version %s"
)
logger_args = [
self.__class__.__name__,
f.__name__,
eol_version,
)
]
if new_target:
messsage += " . Use the following API endpoint instead: %s"
logger_args.append(new_target)
logger.warning(messsage, *logger_args)
return f(self, *args, **kwargs)

return functools.update_wrapper(wraps, f)
Expand Down
49 changes: 27 additions & 22 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
@has_access_api
@event_logger.log_this
@expose("/datasources/")
@deprecated()
@deprecated(new_target="api/v1/dataset/")
def datasources(self) -> FlaskResponse:
return self.json_response(
sorted(
Expand Down Expand Up @@ -505,7 +505,7 @@ def generate_json(
@expose("/slice_json/<int:slice_id>")
@etag_cache()
@check_resource_permissions(check_slice_perms)
@deprecated()
@deprecated(new_target="/api/v1/chart/data")
def slice_json(self, slice_id: int) -> FlaskResponse:
form_data, slc = get_form_data(slice_id, use_slice_data=True)
if not slc:
Expand All @@ -529,7 +529,7 @@ def slice_json(self, slice_id: int) -> FlaskResponse:
@has_access_api
@event_logger.log_this
@expose("/annotation_json/<int:layer_id>")
@deprecated()
@deprecated(new_target="/api/v1/chart/data")
def annotation_json( # pylint: disable=no-self-use
self, layer_id: int
) -> FlaskResponse:
Expand Down Expand Up @@ -999,7 +999,10 @@ def explore(
@has_access_api
@event_logger.log_this
@expose("/filter/<datasource_type>/<int:datasource_id>/<column>/")
@deprecated()
@deprecated(
new_target="/api/v1/datasource/<datasource_type>/"
"<datasource_id>/column/<column_name>/values/"
)
def filter( # pylint: disable=no-self-use
self, datasource_type: str, datasource_id: int, column: str
) -> FlaskResponse:
Expand Down Expand Up @@ -1143,7 +1146,7 @@ def save_or_overwrite_slice(
@event_logger.log_this
@expose("/tables/<int:db_id>/<schema>/")
@expose("/tables/<int:db_id>/<schema>/<force_refresh>/")
@deprecated()
@deprecated(new_target="api/v1/database/<int:pk>/tables/")
def tables( # pylint: disable=no-self-use
self,
db_id: int,
Expand Down Expand Up @@ -1352,7 +1355,7 @@ def add_slices( # pylint: disable=no-self-use
@has_access_api
@event_logger.log_this
@expose("/testconn", methods=["POST", "GET"])
@deprecated()
@deprecated(new_target="/api/v1/database/test_connection/")
def testconn(self) -> FlaskResponse: # pylint: disable=no-self-use
"""Tests a sqla connection"""
db_name = request.json.get("name")
Expand Down Expand Up @@ -1441,7 +1444,7 @@ def get_user_activity_access_error(user_id: int) -> Optional[FlaskResponse]:
@has_access_api
@event_logger.log_this
@expose("/recent_activity/<int:user_id>/", methods=["GET"])
@deprecated()
@deprecated(new_target="/api/v1/log/recent_activity/<user_id>/")
def recent_activity(self, user_id: int) -> FlaskResponse:
"""Recent activity (actions) for a given user"""
error_obj = self.get_user_activity_access_error(user_id)
Expand All @@ -1462,7 +1465,7 @@ def recent_activity(self, user_id: int) -> FlaskResponse:
@has_access_api
@event_logger.log_this
@expose("/available_domains/", methods=["GET"])
@deprecated()
@deprecated(new_target="/api/v1/available_domains/")
def available_domains(self) -> FlaskResponse: # pylint: disable=no-self-use
"""
Returns the list of available Superset Webserver domains (if any)
Expand All @@ -1477,7 +1480,7 @@ def available_domains(self) -> FlaskResponse: # pylint: disable=no-self-use
@has_access_api
@event_logger.log_this
@expose("/fave_dashboards_by_username/<username>/", methods=["GET"])
@deprecated()
@deprecated(new_target="api/v1/dashboard/favorite_status/")
def fave_dashboards_by_username(self, username: str) -> FlaskResponse:
"""This lets us use a user's username to pull favourite dashboards"""
user = security_manager.find_user(username=username)
Expand All @@ -1487,7 +1490,7 @@ def fave_dashboards_by_username(self, username: str) -> FlaskResponse:
@has_access_api
@event_logger.log_this
@expose("/fave_dashboards/<int:user_id>/", methods=["GET"])
@deprecated()
@deprecated(new_target="api/v1/dashboard/favorite_status/")
def fave_dashboards(self, user_id: int) -> FlaskResponse:
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
Expand Down Expand Up @@ -1524,7 +1527,7 @@ def fave_dashboards(self, user_id: int) -> FlaskResponse:
@has_access_api
@event_logger.log_this
@expose("/created_dashboards/<int:user_id>/", methods=["GET"])
@deprecated()
@deprecated(new_target="api/v1/dashboard/")
def created_dashboards(self, user_id: int) -> FlaskResponse:
error_obj = self.get_user_activity_access_error(user_id)
if error_obj:
Expand Down Expand Up @@ -1609,7 +1612,7 @@ def user_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
@event_logger.log_this
@expose("/created_slices", methods=["GET"])
@expose("/created_slices/<int:user_id>/", methods=["GET"])
@deprecated()
@deprecated(new_target="api/v1/chart/")
def created_slices(self, user_id: Optional[int] = None) -> FlaskResponse:
"""List of slices created by this user"""
if not user_id:
Expand Down Expand Up @@ -1926,7 +1929,7 @@ def log(self) -> FlaskResponse: # pylint: disable=no-self-use
@has_access
@expose("/get_or_create_table/", methods=["POST"])
@event_logger.log_this
@deprecated()
@deprecated(new_target="api/v1/dataset/get_or_create/")
def sqllab_table_viz(self) -> FlaskResponse: # pylint: disable=no-self-use
"""Gets or creates a table object with attributes passed to the API.

Expand Down Expand Up @@ -2041,7 +2044,9 @@ def sqllab_viz(self) -> FlaskResponse: # pylint: disable=no-self-use
@has_access
@expose("/extra_table_metadata/<int:database_id>/<table_name>/<schema>/")
@event_logger.log_this
@deprecated()
@deprecated(
new_target="api/v1/database/<int:pk>/table_extra/<table_name>/<schema_name>/"
)
def extra_table_metadata( # pylint: disable=no-self-use
self, database_id: int, table_name: str, schema: str
) -> FlaskResponse:
Expand Down Expand Up @@ -2099,7 +2104,7 @@ def theme(self) -> FlaskResponse:
@has_access_api
@expose("/results/<key>/")
@event_logger.log_this
@deprecated()
@deprecated(new_target="/api/v1/sqllab/results/")
def results(self, key: str) -> FlaskResponse:
return self.results_exec(key)

Expand Down Expand Up @@ -2221,7 +2226,7 @@ def results_exec(key: str) -> FlaskResponse:
on_giveup=lambda details: db.session.rollback(),
max_tries=5,
)
@deprecated()
@deprecated(new_target="/api/v1/query/stop")
def stop_query(self) -> FlaskResponse:
client_id = request.form.get("client_id")
query = db.session.query(Query).filter_by(client_id=client_id).one()
Expand Down Expand Up @@ -2250,7 +2255,7 @@ def stop_query(self) -> FlaskResponse:
@has_access_api
@event_logger.log_this
@expose("/validate_sql_json/", methods=["POST", "GET"])
@deprecated()
@deprecated(new_target="/api/v1/database/<pk>/validate_sql/")
def validate_sql_json(
# pylint: disable=too-many-locals,no-self-use
self,
Expand Down Expand Up @@ -2323,7 +2328,7 @@ def validate_sql_json(
@handle_api_exception
@event_logger.log_this
@expose("/sql_json/", methods=["POST"])
@deprecated()
@deprecated(new_target="/api/v1/sqllab/execute/")
def sql_json(self) -> FlaskResponse:
errors = SqlJsonPayloadSchema().validate(request.json)
if errors:
Expand Down Expand Up @@ -2401,7 +2406,7 @@ def _create_response_from_execution_context( # pylint: disable=invalid-name, no
@has_access
@event_logger.log_this
@expose("/csv/<client_id>")
@deprecated()
@deprecated(new_target="/api/v1/sqllab/export/")
def csv(self, client_id: str) -> FlaskResponse: # pylint: disable=no-self-use
"""Download the query results as csv."""
logger.info("Exporting CSV file [%s]", client_id)
Expand Down Expand Up @@ -2475,7 +2480,7 @@ def csv(self, client_id: str) -> FlaskResponse: # pylint: disable=no-self-use
@has_access
@event_logger.log_this
@expose("/fetch_datasource_metadata")
@deprecated()
@deprecated(new_target="api/v1/database/<int:pk>/table/<table_name>/<schema_name>/")
def fetch_datasource_metadata(self) -> FlaskResponse: # pylint: disable=no-self-use
"""
Fetch the datasource metadata.
Expand All @@ -2498,7 +2503,7 @@ def fetch_datasource_metadata(self) -> FlaskResponse: # pylint: disable=no-self
@event_logger.log_this
@expose("/queries/<float:last_updated_ms>")
@expose("/queries/<int:last_updated_ms>")
@deprecated()
@deprecated(new_target="api/v1/query/updated_since")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but I noticed that some of these new targets have trailing slashes and some don't. is that the way the api is set up, do you know?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On FAB ModelRestApi CRUD endpoints are created given the following logic:

  • All endpoints at the root of a resource name have a trailing slash, example:
    GET /api/v1/dataset/
    POST /api/v1/dataset/
  • All endpoints that contain a path parameter don't have a trailing slash, example:
    GET /api/v1/dataset/1
    PUT /api/v1/dataset/1

On related nested resource names:
GET /api/v1/dataset/1/column/
GET /api/v1/dataset/1/column/1

This enforces a base cohesion, yet not a global cohesion. Currently on Superset theres probably some confusion around this. I'll take a pass on the most recent migrations, to check if there's some obvious changes ASAP

def queries(self, last_updated_ms: Union[float, int]) -> FlaskResponse:
"""
Get the updated queries.
Expand Down Expand Up @@ -2530,7 +2535,7 @@ def queries_exec(last_updated_ms: Union[float, int]) -> FlaskResponse:
@has_access
@event_logger.log_this
@expose("/search_queries")
@deprecated()
@deprecated(new_target="api/v1/query/")
def search_queries(self) -> FlaskResponse: # pylint: disable=no-self-use
"""
Search for previously run sqllab queries. Used for Sqllab Query Search
Expand Down