You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "betodealmeida (via GitHub)" <gi...@apache.org> on 2023/02/23 17:17:34 UTC
[GitHub] [superset] betodealmeida opened a new pull request, #23174: feat: database extension registry
betodealmeida opened a new pull request, #23174:
URL: https://github.com/apache/superset/pull/23174
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Allow the implementation of custom extensions to customize databases via `setupExtensions.ts`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
WIP
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Writing tests.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #23174: feat: database extension registry
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #23174:
URL: https://github.com/apache/superset/pull/23174#discussion_r1140855090
##########
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx:
##########
@@ -566,6 +602,93 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const [useSSHTunneling, setUseSSHTunneling] = useState<boolean>(false);
+ const createPostProcessingCallbacks = useRef<any>({});
+ const updatePostProcessingCallbacks = useRef<any>({});
Review Comment:
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.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #23174: feat: database extension registry
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #23174:
URL: https://github.com/apache/superset/pull/23174#discussion_r1178477002
##########
superset-frontend/src/features/databases/DatabaseModal/styles.ts:
##########
@@ -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;
Review Comment:
fixing this height so that it doesn't jump when loading.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #23174: feat: database extension registry
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23174:
URL: https://github.com/apache/superset/pull/23174#issuecomment-1442650670
# [Codecov](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#23174](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c1e20f5) into [master](https://codecov.io/gh/apache/superset/commit/95eb8d79d094315bc7bed97666f036f32a7ffc4a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (95eb8d7) will **increase** coverage by `0.02%`.
> The diff coverage is `8.57%`.
> :exclamation: Current head c1e20f5 differs from pull request most recent head 238a3a0. Consider uploading reports for the commit 238a3a0 to get more accurate results
```diff
@@ Coverage Diff @@
## master #23174 +/- ##
==========================================
+ Coverage 67.45% 67.48% +0.02%
==========================================
Files 1898 1898
Lines 73111 73020 -91
Branches 7952 7904 -48
==========================================
- Hits 49320 49275 -45
+ Misses 21755 21719 -36
+ Partials 2036 2026 -10
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.63% <8.57%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../packages/superset-ui-core/src/chart/types/Base.ts](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY2hhcnQvdHlwZXMvQmFzZS50cw==) | `100.00% <ø> (ø)` | |
| [...set-ui-core/src/ui-overrides/ExtensionsRegistry.ts](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvdWktb3ZlcnJpZGVzL0V4dGVuc2lvbnNSZWdpc3RyeS50cw==) | `100.00% <ø> (ø)` | |
| [...ackages/superset-ui-core/src/utils/featureFlags.ts](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvdXRpbHMvZmVhdHVyZUZsYWdzLnRz) | `100.00% <ø> (ø)` | |
| [...gins/legacy-plugin-chart-world-map/src/WorldMap.js](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvV29ybGRNYXAuanM=) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/BigNumber/BigNumberViz.tsx](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQmlnTnVtYmVyL0JpZ051bWJlclZpei50c3g=) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQm94UGxvdC9FY2hhcnRzQm94UGxvdC50c3g=) | `0.00% <0.00%> (ø)` | |
| [.../plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvRnVubmVsL0VjaGFydHNGdW5uZWwudHN4) | `0.00% <0.00%> (ø)` | |
| [...ns/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR2F1Z2UvRWNoYXJ0c0dhdWdlLnRzeA==) | `0.00% <0.00%> (ø)` | |
| [...ns/plugin-chart-echarts/src/Graph/EchartsGraph.tsx](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR3JhcGgvRWNoYXJ0c0dyYXBoLnRzeA==) | `0.00% <0.00%> (ø)` | |
| [...rts/src/MixedTimeseries/EchartsMixedTimeseries.tsx](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL0VjaGFydHNNaXhlZFRpbWVzZXJpZXMudHN4) | `0.00% <0.00%> (ø)` | |
| ... and [36 more](https://codecov.io/gh/apache/superset/pull/23174?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #23174: feat: database extension registry
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #23174:
URL: https://github.com/apache/superset/pull/23174#discussion_r1140839425
##########
superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx:
##########
@@ -437,6 +458,33 @@ const ExtraOptions = ({
</StyledInputContainer>
)}
</Collapse.Panel>
+ {dbConfigExtensions?.map?.(extension => {
+ const Extension =
+ extension.component as FunctionComponent<IExtensionProps>;
+ return (
+ <Collapse.Panel
+ header={
+ <div>
+ {extension.logo ? (
+ <img
+ alt="dbt Cloud"
Review Comment:
is this alt meant to be hard coded?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #23174: feat: database extension registry
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #23174:
URL: https://github.com/apache/superset/pull/23174#discussion_r1179498111
##########
superset-frontend/src/features/databases/DatabaseModal/index.tsx:
##########
@@ -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);
Review Comment:
we can remove this
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a diff in pull request #23174: feat: database extension registry
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida commented on code in PR #23174:
URL: https://github.com/apache/superset/pull/23174#discussion_r1142374297
##########
superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx:
##########
@@ -437,6 +458,33 @@ const ExtraOptions = ({
</StyledInputContainer>
)}
</Collapse.Panel>
+ {dbConfigExtensions?.map?.(extension => {
+ const Extension =
+ extension.component as FunctionComponent<IExtensionProps>;
+ return (
+ <Collapse.Panel
+ header={
+ <div>
+ {extension.logo ? (
+ <img
+ alt="dbt Cloud"
Review Comment:
Ah, sorry, that needs to go.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida merged pull request #23174: feat: database extension registry
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida merged PR #23174:
URL: https://github.com/apache/superset/pull/23174
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a diff in pull request #23174: feat: database extension registry
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida commented on code in PR #23174:
URL: https://github.com/apache/superset/pull/23174#discussion_r1152616471
##########
superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx:
##########
@@ -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';
Review Comment:
This is no longer needed and is causing tests to fail.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org