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.

Sign up for GitHub

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

Jump to bottom

fix: The dynamic form to connect to Snowflake DB is not returning any errors #20013

Merged
merged 2 commits into from
May 13, 2022
Merged

fix: The dynamic form to connect to Snowflake DB is not returning any errors #20013

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc138f3
fix: The dynamic form to connect to Snowflake DB is not returning any…
diegomedina248 May 10, 2022
768cecf
PR comment
diegomedina248 May 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 26 additions & 16 deletions superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,13 @@ import {
} from './styles';
import ModalHeader, { DOCUMENTATION_LINK } from './ModalHeader';

enum Engines {
GSheet = 'gsheets',
Snowflake = 'snowflake',
}

const engineSpecificAlertMapping = {
gsheets: {
[Engines.GSheet]: {
message: 'Why do I need to create a database?',
description:
'To begin using your Google Sheets, you need to create a database first. ' +
Expand All @@ -109,8 +114,6 @@ const TabsStyled = styled(Tabs)`
}
`;

const googleSheetConnectionEngine = 'gsheets';

interface DatabaseModalProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
Expand Down Expand Up @@ -586,7 +589,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
// cast the new encrypted extra object into a string
dbToUpdate.encrypted_extra = JSON.stringify(additionalEncryptedExtra);
// this needs to be added by default to gsheets
if (dbToUpdate.engine === 'gsheets') {
if (dbToUpdate.engine === Engines.GSheet) {
dbToUpdate.impersonate_user = true;
}
}
Expand All @@ -605,8 +608,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
dbToUpdate.extra = serializeExtra(dbToUpdate?.extra_json);
}

setLoading(true);
if (db?.id) {
setLoading(true);
const result = await updateResource(
db.id as number,
dbToUpdate as DatabaseObject,
Expand All @@ -621,7 +624,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
}
} else if (db) {
// Create
setLoading(true);
const dbId = await createResource(
dbToUpdate as DatabaseObject,
dbToUpdate.configuration_method === CONFIGURATION_METHOD.DYNAMIC_FORM, // onShow toast on SQLA Forms
Expand All @@ -636,14 +638,14 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
addSuccessToast(t('Database connected'));
}
}
}

// Import - doesn't use db state
if (!db) {
setLoading(true);
} else {
// Import - doesn't use db state
setImportingModal(true);

if (!(fileList[0].originFileObj instanceof File)) return;
if (!(fileList[0].originFileObj instanceof File)) {
return;
}

const dbId = await importResource(
fileList[0].originFileObj,
passwords,
Expand Down Expand Up @@ -1083,15 +1085,23 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({

// eslint-disable-next-line consistent-return
const errorAlert = () => {
let alertErrors: string[] = [];
if (isEmpty(dbErrors) === false) {
const message: Array<string> =
typeof dbErrors === 'object' ? Object.values(dbErrors) : [];
alertErrors = typeof dbErrors === 'object' ? Object.values(dbErrors) : [];
} else if (db?.engine === Engines.Snowflake) {
alertErrors =
validationErrors?.error_type === 'GENERIC_DB_ENGINE_ERROR'
? [validationErrors?.description]
: [];
}

if (alertErrors.length) {
return (
<Alert
type="error"
css={(theme: SupersetTheme) => antDErrorAlertStyles(theme)}
message={t('Database Creation Error')}
description={message?.[0] || dbErrors}
description={t(alertErrors[0])}
Copy link
Member

Choose a reason for hiding this comment

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

Looking above at line 1095, it seems there's a condition where alertErrors might be [], in which case this would be translating/displaying undefined right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we're checking for content in 1098

Copy link
Member

Choose a reason for hiding this comment

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

lol 🤦

/>
);
}
Expand Down Expand Up @@ -1523,7 +1533,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
validationErrors={validationErrors}
/>
<div css={(theme: SupersetTheme) => infoTooltip(theme)}>
{dbModel.engine !== googleSheetConnectionEngine && (
{dbModel.engine !== Engines.GSheet && (
<>
<Button
data-test="sqla-connect-btn"
Expand Down