dbeaver/pro#8821 feat: add import settings step - #4506
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 43 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…-with-optional-snapshotrollback-safety-net
…mergeupsert-with-optional-snapshotrollback-safety-net' into 8821-customer-wants-import-with-mergeupsert-with-optional-snapshotrollback-safety-net
SychevAndrey
changed the title
| function getDefaultSettings(configuration: IDataImportDriverConfiguration): DataTransferImportSettings { | ||
| const settings: DataTransferImportSettings = {}; | ||
|
|
||
| if (configuration.supportsTransactions) { | ||
| settings.useTransactions = true; | ||
| } | ||
|
|
||
| if (configuration.supportedInsertReplaceMethods) { | ||
| settings.onDuplicateKeyMethod = undefined; | ||
| } | ||
|
|
||
| if (configuration.supportsBulkLoad) { | ||
| settings.useBulkLoad = false; | ||
| } | ||
|
|
||
| return settings; | ||
| } |
There was a problem hiding this comment.
it would be nice to have 1 interface with settings so don't need to map it at all
There was a problem hiding this comment.
i got it why this happens:
because getting data and saving data has different interfaces on backend:
dataTransferImportDataIntoResults - saving
IDataImportDriverConfiguration- getting in resource
|
|
||
| export interface IDataImportDialogPayload { | ||
| tableName: string; | ||
| projectId: string; |
There was a problem hiding this comment.
connectionKey: IConnectionInfoParams?
| DataImportDialog, | ||
| DataImportDriverConfigurationResource, | ||
| createConnectionParam(payload.projectId, payload.connectionId), | ||
| { silent: true }, |
There was a problem hiding this comment.
I believe that those setting are kinda optional, so if something goes wrong, we can just skip it. If we show an error or smth, we should think about an action the user can do. Retry to get the driver configuration or what? I would let them go and try import with default settings. And if there is a real problem, they will se an error there
There was a problem hiding this comment.
Okay, but its better to default to no options for useResource and .data for getting data from resource
| { silent: true }, | ||
| ); | ||
|
|
||
| const driverConfiguration = driverConfigurationResource.tryGetData ?? null; |
There was a problem hiding this comment.
do we have specific behaviour here? why tryGetData instead of .data?
There was a problem hiding this comment.
see the answer above
…-with-optional-snapshotrollback-safety-net
…-with-optional-snapshotrollback-safety-net
…-with-optional-snapshotrollback-safety-net
…-with-optional-snapshotrollback-safety-net
…mergeupsert-with-optional-snapshotrollback-safety-net' into 8821-customer-wants-import-with-mergeupsert-with-optional-snapshotrollback-safety-net
…-with-optional-snapshotrollback-safety-net
…mergeupsert-with-optional-snapshotrollback-safety-net' into 8821-customer-wants-import-with-mergeupsert-with-optional-snapshotrollback-safety-net
5 participants
No description provided.