-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: add datasource config in import module #509
Conversation
return _delete(`/api/datasources/${id}`)(undefined, config); | ||
}, | ||
batchDeleteDatasource: (payload, config?) => { | ||
return _delete(`/api/datasources`)(undefined, { data: payload }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recommand: undefined => {}?
app/interfaces/import.ts
Outdated
endpoint: string; | ||
accessKey: string; | ||
accessSecret: string; | ||
bucket?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is required now?
accessSecret: string; | ||
bucket?: string; | ||
token?: string; | ||
key: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the token and key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are the parameters of the importer, key is the file path
keyFile?: string; | ||
keyData?: string; | ||
passPhrase?: string; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are they used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the configuration definition the same as the nebula importer, but it is not currently used in the studio
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they are not used in our app, you could not pre-define them now, maybe we will never use these fields, it's little redundant.
<FormItem name="platform" rules={[{ required: true, message: intl.get('formRules.platformRequired') } ]}> | ||
<Select placeholder={intl.get('import.selectPlatform')}> | ||
<Select.Option value="aws">AWS S3</Select.Option> | ||
<Select.Option value="aliyun">OSS</Select.Option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aliyun OSS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tecent COS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
S3 is a standard, also should consider the local s3 service
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recommend to use one component per file, easy to understand
return nil, ecode.WithBadRequest(err, "json stringify error") | ||
return nil, ecode.WithErrorMessage(ecode.ErrBadRequest, err, "json stringify error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WithBadRequest is more short and proper?
bucket = pathParts[1] | ||
} | ||
} else { | ||
// Format: https://<bucket-name>.s3.<region>.amazonaws.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can it also work for aliyun oss?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split the oss logic out from here
} | ||
defer conn.Close() | ||
|
||
// create an SFTP client session | ||
client, err := sftp.NewClient(conn) | ||
if err != nil { | ||
return ecode.WithBadRequest(err, "failed to create SFTP session") | ||
return ecode.WithErrorMessage(ecode.ErrBadRequest, err, "failed to create SFTP session") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ecode.WithBadRquest is the short-mode of ecode.WithErrorMessage(ecode.ErrBadRequest....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@veezhang why are they different? I previously think that the WithBadRequest
is the shortcut of ht WithErrorMessage(ecode.ErrBadRequest...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
app/interfaces/datasource.ts
Outdated
export enum IDatasourceType { | ||
's3' = 's3', | ||
'sftp' = 'sftp', | ||
'local' = 'local' | ||
} | ||
export enum IS3Platform { | ||
'aws' = 'aws', | ||
'oss' = 'oss', | ||
'customize' = 'customize', | ||
'tecent' = 'cos' | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export enum DatasourceType {
S3 = 's3',
SFTP = 'sftp',
Local = 'local'
}
export enum S3Platform {
AWS = 'aws',
OSS = 'oss',
Tecent = 'cos',
Customize = 'customize',
}
app/interfaces/datasource.ts
Outdated
export interface IDatasourceAdd { | ||
name: string; | ||
type: IDatasourceType; | ||
platform?: string; | ||
s3Config?: { | ||
accessKey: string; | ||
accessSecret?: string; | ||
endpoint: string; | ||
bucket: string; | ||
region?: string; | ||
}; | ||
sftpConfig?: { | ||
host: string; | ||
port: number; | ||
username: string; | ||
password: string; | ||
}; | ||
} | ||
export interface IDatasourceUpdate extends IDatasourceAdd { | ||
id: number; | ||
} | ||
|
||
export interface IDatasourceItem { | ||
id?: number; | ||
name: string; | ||
type: IDatasourceType; | ||
platform?: string; | ||
createTime?: string; | ||
s3Config?: { | ||
accessKey: string; | ||
accessSecret?: string; | ||
endpoint: string; | ||
bucket: string; | ||
region?: string; | ||
}; | ||
sftpConfig?: { | ||
host: string; | ||
port: number; | ||
username: string; | ||
password: string; | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repetitive content between IDatasourceAdd
and IDatasourceItem
const submit = async (values: IDatasourceItem) => { | ||
const _type = values.type || type; | ||
setLoading(true); | ||
if(mode === 'create') { | ||
const flag = await addDataSource({ | ||
type: _type, | ||
name: '', | ||
...values | ||
}); | ||
setLoading(false); | ||
flag && (message.success(intl.get('schema.createSuccess')), onConfirm()); | ||
} else { | ||
let _config; | ||
switch (_type) { | ||
case IDatasourceType.s3: | ||
_config = values.s3Config; | ||
_config.accessSecret === tempPwd && delete _config.accessSecret; | ||
break; | ||
case IDatasourceType.sftp: | ||
_config = values.sftpConfig; | ||
_config.password === tempPwd && delete _config.password; | ||
break; | ||
default: | ||
break; | ||
} | ||
const flag = await updateDataSource({ | ||
id: data.id, | ||
type: _type, | ||
name: '', | ||
...values | ||
}); | ||
setLoading(false); | ||
flag && (message.success(intl.get('common.updateSuccess')), onConfirm()); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const submit = async (values: IDatasourceItem) => {
setLoading(true);
const payload = { type: values.type || type, name: '', ...values };
if (mode === 'create') {
const flag = await addDataSource(payload);
setLoading(false);
flag && (message.success(intl.get('common.updateSuccess')), onConfirm());
return;
}
switch (payload.type) {
case IDatasourceType.s3: {
const c = payload.s3Config;
payload.s3Config = c?.accessSecret === tempPwd ? { ...c, accessSecret: undefined } : c;
break;
}
case IDatasourceType.sftp: {
const c = payload.sftpConfig;
payload.sftpConfig = c?.password === tempPwd ? { ...c, password: undefined } : c;
break;
}
default:
break;
}
const flag = await updateDataSource({ id: data.id, ...payload });
// const flag = await (() => {
// if (mode === 'create') {
// return addDataSource(payload);
// }
// switch (payload.type) {
// case IDatasourceType.s3: {
// const c = payload.s3Config;
// payload.s3Config = c?.accessSecret === tempPwd ? { ...c, accessSecret: undefined } : c;
// break;
// }
// case IDatasourceType.sftp: {
// const c = payload.sftpConfig;
// payload.sftpConfig = c?.password === tempPwd ? { ...c, password: undefined } : c;
// break;
// }
// default:
// break;
// }
// return updateDataSource({ id: data.id, ...payload });
// })();
setLoading(false);
flag && (message.success(intl.get('common.updateSuccess')), onConfirm());
};
const cloudKeys = [ | ||
{ | ||
title: 'ipAddress', | ||
key: ['s3Config', 'endpoint'] | ||
}, | ||
{ | ||
title: 'bucketName', | ||
key: ['s3Config', 'bucket'] | ||
}, | ||
{ | ||
title: 'accessKeyId', | ||
key: ['s3Config', 'accessKey'] | ||
}, | ||
{ | ||
title: 'region', | ||
key: ['s3Config', 'region'] | ||
}, | ||
{ | ||
title: 'createTime', | ||
key: 'createTime', | ||
render: data => data && dayjs(data).format('YYYY-MM-DD HH:mm:ss') | ||
}, | ||
]; | ||
const sftpKeys = [{ | ||
title: 'ipAddress', | ||
key: ['sftpConfig', 'host'], | ||
render: (_, row) => `${row.sftpConfig.host}:${row.sftpConfig.port}` | ||
}, | ||
{ | ||
title: 'account', | ||
key: ['sftpConfig', 'username'] | ||
}, | ||
{ | ||
title: 'createTime', | ||
key: 'createTime', | ||
render: data => data && dayjs(data).format('YYYY-MM-DD HH:mm:ss') | ||
} | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const cloudColumns: TableColumnType<IDatasourceItem>[] = [
{
title: <Translation>{(i) => i.get('import.ipAddress')}</Translation>,
dataIndex: ['s3Config', 'endpoint'],
},
{
title: <Translation>{(i) => i.get('import.bucketName')}</Translation>,
dataIndex: ['s3Config', 'bucket'],
},
{
title: <Translation>{(i) => i.get('import.accessKeyId')}</Translation>,
dataIndex: ['s3Config', 'accessKey'],
},
{
title: <Translation>{(i) => i.get('import.region')}</Translation>,
dataIndex: ['s3Config', 'region'],
},
{
title: <Translation>{(i) => i.get('import.createTime')}</Translation>,
dataIndex: 'createTime',
render: (data) => data && dayjs(data).format('YYYY-MM-DD HH:mm:ss'),
},
];
const sftpColumns: TableColumnType<IDatasourceItem>[] = [
{
title: <Translation>{(i) => i.get('import.ipAddress')}</Translation>,
dataIndex: ['sftpConfig', 'host'],
render: (_, row) => `${row.sftpConfig.host}:${row.sftpConfig.port}`,
},
{
title: <Translation>{(i) => i.get('import.account')}</Translation>,
dataIndex: ['sftpConfig', 'username'],
},
{
title: <Translation>{(i) => i.get('import.createTime')}</Translation>,
dataIndex: 'createTime',
render: (data) => data && dayjs(data).format('YYYY-MM-DD HH:mm:ss'),
},
];
const columns = columnKeys[type].map(item => ({ | ||
title: intl.get(`import.${item.title}`), | ||
dataIndex: item.key, | ||
render: item.render | ||
})).concat(({ | ||
title: intl.get('common.operation'), | ||
key: 'operation', | ||
render: (_, item: IDatasourceItem) => (<div className={styles.operation}> | ||
<Button className="primaryBtn" onClick={() => editItem(item)}> | ||
<Icon type="icon-studio-btn-detail" /> | ||
</Button> | ||
<Popconfirm | ||
onConfirm={() => deleteItem(item.id)} | ||
title={intl.get('common.ask')} | ||
okText={intl.get('common.confirm')} | ||
cancelText={intl.get('common.cancel')} | ||
> | ||
<Button className="warningBtn"> | ||
<Icon type="icon-studio-btn-delete" /> | ||
</Button> | ||
</Popconfirm> | ||
</div>) | ||
} as any)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const tableColumns: TableColumnType<IDatasourceItem>[] = useMemo(() => {
const columns = type === IDatasourceType.s3 ? cloudColumns : sftpColumns;
return columns.concat({
title: intl.get('common.operation'),
key: 'operation',
render: (_, item) => (
<div className={styles.operation}>
<Button className="primaryBtn" onClick={() => editItem(item)}>
<Icon type="icon-studio-btn-detail" />
</Button>
<Popconfirm
onConfirm={() => deleteItem(item.id)}
title={intl.get('common.ask')}
okText={intl.get('common.confirm')}
cancelText={intl.get('common.cancel')}
>
<Button className="warningBtn">
<Icon type="icon-studio-btn-delete" />
</Button>
</Popconfirm>
</div>
),
});
}, [type, intl]);
const _path = path.slice(0, -1).split('/'); | ||
_path.pop(); | ||
const newPath = _path.join('/').length ? _path.join('/') + '/' : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* - `/a/b/c/?` => `/a/b/`
* - `a/b/?` => `a`
* - `a/?` => ``
*/
const parentPath = path.replace(/(\/|^)[^/]+\/?$/, '$1');
const handleConfirm = useCallback(async () => { | ||
if(activeItem && activeId === IDatasourceType.local) { | ||
// select local file | ||
onConfirm(activeItem, state); | ||
} else { | ||
setState({ loading: true }); | ||
const _path = `${path === '/' ? '' : path}${activeItem.name}`; | ||
const data = await previewFile({ id: activeId, path: _path }); | ||
const item = { | ||
name: activeItem.name, | ||
withHeader: false, | ||
delimiter: ',', | ||
sample: data.contents.join('\r\n'), | ||
path: _path, | ||
datasourceId: activeId === IDatasourceType.local ? null : activeId, | ||
} as any; | ||
setState({ loading: false }); | ||
onConfirm(item, state); | ||
} | ||
}, [activeItem, activeId, path]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
early return, unnecessary else
& fix some issues
datasource model
b77ed0a
to
6d10007
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Description:
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc: