-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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(db-connection-ui): Show Preferred DBs #14951
feat(db-connection-ui): Show Preferred DBs #14951
Conversation
* poc picker for db selection * working select * setup is loading for available dbs and step1 view * fix on close * update on fetch * remove unneeded code * add some styls
…-io/superset into pexdax/db-connection-ui-show-preferred
Codecov Report
@@ Coverage Diff @@
## pexdax/db-connection-ui #14951 +/- ##
===========================================================
+ Coverage 77.52% 77.63% +0.11%
===========================================================
Files 965 964 -1
Lines 49530 49361 -169
Branches 6272 6229 -43
===========================================================
- Hits 38398 38322 -76
+ Misses 10929 10840 -89
+ Partials 203 199 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
a380629
to
d5c5167
Compare
@@ -1075,10 +1075,10 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( | |||
# use the "engine_name" attribute of the corresponding DB engine spec | |||
# in `superset/db_engine_specs/`. | |||
PREFERRED_DATABASES: List[str] = [ |
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.
will remove before merging
/testenv up |
@hughhhh Container image not yet published for this PR. Please try again when build is complete. |
@hughhhh Ephemeral environment creation failed. Please check the Actions logs for details. |
@@ -340,5 +355,30 @@ export const EditHeaderSubtitle = styled.div` | |||
`; | |||
|
|||
export const SelectDatabaseStyles = styled.div` | |||
margin: ${({ theme }) => theme.gridUnit * 4}px; | |||
margin: 0 16px; |
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.
use gridUnit
@@ -299,6 +301,73 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |||
} | |||
}; | |||
|
|||
const setDatabaseModel = 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.
can the param be "engine" if it's just a string? Maybe a type on that would help, too.
engine: option, | ||
}, | ||
}); | ||
console.log('isDynamic', isDynamic); |
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.
nit remove
console.log('isDynamic', isDynamic); | ||
}; | ||
|
||
const renderAvailableSelector = () => ( |
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 we move step one into a separate file?
|
||
const renderModalFooter = () => | ||
db // if db show back + connect | ||
? [ |
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.
db && (
<>
would that work instead?
This is looking good @hughhhh! |
SUMMARY
Added starter Step 1 for preferred dbs render. User will now see the preferred DBs set in the superset_config file, and will be in a top section to make it easier for people to get to the connection page (step 2)
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
https://www.loom.com/share/ab848326e2884b19bd248303e50f4da0
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION