You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:40:03 UTC
[GitHub] [incubator-superset] nytai commented on a change in pull request #10442: refactor(listviews): use correct filter endpoints for charts and datasets
nytai commented on a change in pull request #10442:
URL: https://github.com/apache/incubator-superset/pull/10442#discussion_r461737272
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -66,78 +66,40 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
-interface Database {
- allow_csv_upload: boolean;
- allow_ctas: boolean;
- allow_cvas: null | boolean;
- allow_dml: boolean;
- allow_multi_schema_metadata_fetch: boolean;
- allow_run_async: boolean;
- allows_cost_estimate: boolean;
- allows_subquery: boolean;
- allows_virtual_table_explore: boolean;
- backend: string;
- database_name: string;
- explore_database_id: number;
- expose_in_sqllab: boolean;
- force_ctas_schema: null | boolean;
- function_names: string[];
- id: number;
-}
-
-const createFetchDatabases = (handleError: (err: Response) => void) => async (
- filterValue = '',
- pageIndex?: number,
- pageSize?: number,
-) => {
- try {
- const queryParams = rison.encode({
- columns: ['database_name', 'id'],
- keys: ['none'],
- ...(pageIndex ? { page: pageIndex } : {}),
- ...(pageSize ? { page_size: pageSize } : {}),
- ...(filterValue ? { filter: filterValue } : {}),
- });
- const { json = {} } = await SupersetClient.get({
- endpoint: `/api/v1/database/?q=${queryParams}`,
- });
-
- return json?.result?.map(
- ({ database_name: label, id: value }: Database) => ({
- label,
- value,
- }),
- );
- } catch (e) {
- handleError(e);
- }
- return [];
-};
export const createFetchSchemas = (
handleError: (error: Response) => void,
) => async (filterValue = '', pageIndex?: number, pageSize?: number) => {
+ // add filters if filterValue
+ const filters = filterValue
+ ? { filters: [{ col: 'schema', opr: 'sw', value: filterValue }] }
+ : {};
try {
const queryParams = rison.encode({
+ columns: ['schema'],
+ keys: ['none'],
...(pageIndex ? { page: pageIndex } : {}),
...(pageSize ? { page_size: pageSize } : {}),
- ...(filterValue ? { filter: filterValue } : {}),
+ ...filters,
Review comment:
good point!
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -66,78 +66,40 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
-interface Database {
- allow_csv_upload: boolean;
- allow_ctas: boolean;
- allow_cvas: null | boolean;
- allow_dml: boolean;
- allow_multi_schema_metadata_fetch: boolean;
- allow_run_async: boolean;
- allows_cost_estimate: boolean;
- allows_subquery: boolean;
- allows_virtual_table_explore: boolean;
- backend: string;
- database_name: string;
- explore_database_id: number;
- expose_in_sqllab: boolean;
- force_ctas_schema: null | boolean;
- function_names: string[];
- id: number;
-}
-
-const createFetchDatabases = (handleError: (err: Response) => void) => async (
- filterValue = '',
- pageIndex?: number,
- pageSize?: number,
-) => {
- try {
- const queryParams = rison.encode({
- columns: ['database_name', 'id'],
- keys: ['none'],
- ...(pageIndex ? { page: pageIndex } : {}),
- ...(pageSize ? { page_size: pageSize } : {}),
- ...(filterValue ? { filter: filterValue } : {}),
- });
- const { json = {} } = await SupersetClient.get({
- endpoint: `/api/v1/database/?q=${queryParams}`,
- });
-
- return json?.result?.map(
- ({ database_name: label, id: value }: Database) => ({
- label,
- value,
- }),
- );
- } catch (e) {
- handleError(e);
- }
- return [];
-};
export const createFetchSchemas = (
handleError: (error: Response) => void,
) => async (filterValue = '', pageIndex?: number, pageSize?: number) => {
+ // add filters if filterValue
+ const filters = filterValue
+ ? { filters: [{ col: 'schema', opr: 'sw', value: filterValue }] }
+ : {};
try {
const queryParams = rison.encode({
+ columns: ['schema'],
+ keys: ['none'],
...(pageIndex ? { page: pageIndex } : {}),
...(pageSize ? { page_size: pageSize } : {}),
- ...(filterValue ? { filter: filterValue } : {}),
+ ...filters,
});
const { json = {} } = await SupersetClient.get({
- endpoint: `/api/v1/database/schemas/?q=${queryParams}`,
+ endpoint: `/api/v1/dataset/?q=${queryParams}`,
});
- return json?.result?.map(
- ({ text: label, value }: { text: string; value: any }) => ({
- label,
- value,
- }),
+ const schemas: string[] = json?.result?.map(
+ ({ schema }: { schema: string }) => schema,
);
+
+ // uniqueify schema values and create options
Review comment:
Building a api route that supports filtering and pagination is a little more involved. Longer term we should definitely do this, but I'm looking for quick fixes in this PR. @dpgaspar do you think it would make sense to add another method for retrieving unique values from some resource, similar to `/api/v1/{resource}/related/{relation}` we could have `/api/v1/{resource}/unique_values/{column}`?
##########
File path: superset-frontend/src/views/CRUD/dataset/AddDatasetModal.tsx
##########
@@ -97,8 +97,13 @@ const DatasetModal: FunctionComponent<DatasetModalProps> = ({
onHide();
})
.catch(
- createErrorHandler(errMsg =>
- addDangerToast(t('Error while saving dataset: %s', errMsg)),
+ createErrorHandler((errMsg: unknown) =>
+ addDangerToast(
+ t(
+ 'Error while saving dataset: %s',
+ (errMsg as { table_name?: string }).table_name,
Review comment:
the error types are currently wrong. Error message can be either a string or an object, in this case the error message is scope to the input. Errors are most likely to occur in table_name field.
Longer terms we should add error messages to the form so the errors appear next to the input that failed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org