-
Notifications
You must be signed in to change notification settings - Fork 14k
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: database extension registry #23174
Conversation
bc80e94
to
238a3a0
Compare
Codecov Report
@@ Coverage Diff @@
## master #23174 +/- ##
==========================================
- Coverage 56.98% 56.97% -0.02%
==========================================
Files 1951 1951
Lines 75412 75448 +36
Branches 8185 8196 +11
==========================================
+ Hits 42976 42984 +8
- Misses 30333 30358 +25
- Partials 2103 2106 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx
Outdated
Show resolved
Hide resolved
@@ -566,6 +602,93 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |||
|
|||
const [useSSHTunneling, setUseSSHTunneling] = useState<boolean>(false); | |||
|
|||
const createPostProcessingCallbacks = useRef<any>({}); | |||
const updatePostProcessingCallbacks = useRef<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.
If I understand this correctly, it looks like the child is sending a function from a lower level component up to the a higher component by attaching it to a ref, is that right? Would it be easier to import the extension into this parent index component, and then you can call the create and update callbacks later in the parent and then pass down just the visual components to the ExtraOptions? That way you don't have to pass the callbacks up to the parent. I'm not sure if that's necessarily the solution, but I know that these extension points are complicated, so trying to help suggest a simpler solution if there is one.
00d00af
to
c09c4a9
Compare
cafea86
to
5f839a8
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.
Nice! I like the simplified approach. I can work on the onsave
method in shell, if you haven't started it.
@@ -16,9 +16,13 @@ | |||
* specific language governing permissions and limitations | |||
* under the License. | |||
*/ | |||
import React, { ChangeEvent, EventHandler } from 'react'; | |||
import React, { ChangeEvent, EventHandler, FunctionComponent } from 'react'; |
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 no longer needed and is causing tests to fail.
5f839a8
to
3597c2c
Compare
3597c2c
to
79eed09
Compare
@@ -573,6 +573,7 @@ export const StyledStickyHeader = styled.div` | |||
top: 0; | |||
z-index: ${({ theme }) => theme.zIndex.max}; | |||
background: ${({ theme }) => theme.colors.grayscale.light5}; | |||
height: ${({ theme }) => theme.gridUnit * 16}px; |
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.
fixing this height so that it doesn't jump when loading.
@@ -715,6 +734,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({ | |||
}; | |||
|
|||
const onSave = async () => { | |||
// TODO: Elizabeth - add some error handling or promise/callback here | |||
dbConfigExtraExtension?.onSave(extraExtensionComponentState, db); |
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.
we can remove this
2790991
to
5a9710d
Compare
fix(dbt cloud): modal header overlap in db connection
Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> Co-authored-by: Lily Kuang <lily@preset.io>
SUMMARY
Allow the implementation of custom extensions to customize databases via
setupExtensions.ts
.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
WIP
TESTING INSTRUCTIONS
Writing tests.
ADDITIONAL INFORMATION