eschutho commented on a change in pull request #14845:
URL: https://github.com/apache/superset/pull/14845#discussion_r644309750



##########
File path: 
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
##########
@@ -162,8 +163,6 @@ function dbReducer(
       };
     case ActionType.fetched:
       return {
-        engine: trimmedState.engine,
-        configuration_method: trimmedState.configuration_method,

Review comment:
       why are we removing these?

##########
File path: 
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
##########
@@ -248,10 +247,6 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> 
= ({
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     const { id, ...update } = db || {};
     if (db?.id) {
-      if (db.sqlalchemy_uri) {

Review comment:
       there won't be a sqlalchemy_uri if you are creating from scratch and 
using the form view.

##########
File path: 
superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts
##########
@@ -85,7 +85,6 @@ export const antDModalNoPaddingStyles = css`
 `;
 
 export const formScrollableStyles = (theme: SupersetTheme) => css`
-  overflow-y: scroll;

Review comment:
       @hughhhh @AAfghahi this will make the header "sticky" right?

##########
File path: superset-frontend/src/views/CRUD/data/database/types.ts
##########
@@ -30,7 +30,15 @@ export type DatabaseObject = {
   created_by?: null | DatabaseUser;
   changed_on_delta_humanized?: string;
   changed_on?: string;
-  parameters?: { database_name?: string };
+  parameters?: {
+    database_name?: string;
+    host?: string;
+    port?: number;
+    engine?: string;

Review comment:
       I believe we moved engine to the root

##########
File path: 
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
##########
@@ -162,8 +163,6 @@ function dbReducer(
       };
     case ActionType.fetched:
       return {
-        engine: trimmedState.engine,
-        configuration_method: trimmedState.configuration_method,

Review comment:
       ok, then you can move the whole case statement down with 
   ```
   case ActionType.dbSelected:
   case ActionType.configMethodChange:
    ```
    since the return value is the same.




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to