refactor(ui): Improving Kafka UI Ingestion Form, Create Domain, Create Secret Modals #6588

jjoyce0510 · 2022-12-01T00:10:53Z

Summary

In this PR, we introduce various improvements to Kafka Ingestion Form, Create Domain Form, Create Secret Form, and UI ingestion more broadly.

Support Form Fields changing the value of the form.. (by passing the FormInstance object as a prop)
Support required / optional Form Fields
Support clearing Form Field selects
AutoPopulate Input Fields once secrets are created (see video below)
Refactor Create Domain Form to correctly support suggested names, and to correctly display 'Create' button
Refactor Create Secret Form to remove use of state, migrate to using Ant forms. Correctly display 'Create' button
Fix: Make secret names more restrictive in type. Exclude special characters.
Fix: Fix Kafka default ingestion source bug (extra ' )
Fix: Improve the copy for tooltips in Kafka source.
Fix: Support PLAINTEXT, SSL security modes in Kafka source.
Migrate Create Domain, Create Secret description inputs to be Input.TextArea.

Screen.Recording.2022-11-30.at.4.01.43.PM.mov

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

…oving-kafka-ui-ingest

github-actions · 2022-12-01T00:47:33Z

Unit Test Results (build & test)

621 tests ±0 617 ✔️ ±0 15m 45s ⏱️ ±0s
157 suites ±0     4 💤 ±0
157 files ±0     0 ❌ ±0

Results for commit 8fdefb6. ± Comparison against base commit 4ca3327.

♻️ This comment has been updated with latest results.

…oving-kafka-ui-ingest

chriscollins3456

looks great to me! i have one question but not stopping from approval. great stuff 👍

chriscollins3456 · 2022-12-01T22:29:13Z

datahub-web-react/src/app/domain/CreateDomainModal.tsx

@@ -88,7 +82,7 @@ export default function CreateDomainModal({ onClose, onCreate }: Props) {
                    <Button onClick={onClose} type="text">
                        Cancel
                    </Button>
-                    <Button id="createDomainButton" onClick={onCreateDomain} disabled={createButtonEnabled}>
+                    <Button id="createDomainButton" onClick={onCreateDomain} disabled={!createButtonEnabled}>


chriscollins3456 · 2022-12-01T22:30:52Z

datahub-web-react/src/app/ingest/secret/SecretBuilderModal.tsx

                            { min: 1, max: 50 },
+                            { pattern: /^[^\s\t${}\\,'"]+$/, message: 'This secret name is not allowed.' },


just some special characters regex?

Yes! We don't want to screw up the injection into the recipe process... We've seen this as an issue before.

chriscollins3456 · 2022-12-01T22:37:57Z

datahub-web-react/src/app/ingest/source/builder/RecipeForm/SecretField/SecretField.tsx

@@ -79,11 +80,16 @@ function SecretFieldTooltip({ tooltipLabel }: { tooltipLabel?: string | ReactNod
    );
 }

-function SecretField({ field, secrets, removeMargin, refetchSecrets }: SecretFieldProps) {
-    const options = secrets.map((secret) => ({ value: `\${${secret.name}}`, label: secret.name }));
+const encodeSecret = (secretName: string) => {


chriscollins3456 · 2022-12-01T22:40:38Z

datahub-web-react/src/app/ingest/source/builder/RecipeForm/common.tsx

-            return updatedRecipe;
-        }
-        set(updatedRecipe, fieldPath, value);
+    if (value === null || value === '' || value === undefined) {


did you find out when undefined gets passed in here?

like I said before, I'm pretty sure this check was when we would pass extra values in here that shouldn't be updated but has since been refactored, so I'm pretty sure this is a safe change to make (also if it's undefined it makes sense we should clear it and its parents if they're empty as well)

I have not found any issues while testing it out locally...

chriscollins3456 · 2022-12-01T22:43:02Z

datahub-web-react/src/app/ingest/source/builder/sources.json

@@ -25,7 +25,7 @@
        "name": "kafka",
        "displayName": "Kafka",
        "docsUrl": "https://datahubproject.io/docs/generated/ingestion/sources/kafka/",
-        "recipe": "source:\n    type: kafka\n    config:\n        connection:\n            consumer_config:\n                security.protocol: \"SASL_SSL\"\n                sasl.mechanism: \"PLAIN\"\n        stateful_ingestion:\n            enabled: true'"
+        "recipe": "source:\n    type: kafka\n    config:\n        connection:\n            consumer_config:\n                security.protocol: \"PLAINTEXT\"\n        stateful_ingestion:\n            enabled: false"


we don't want stateful ingestion enabled by default?

Not for kafka - it currently breaks the source - facepalm.

…e Secret Modals (datahub-project#6588)

jjoyce0510 added 3 commits November 30, 2022 11:58

Adding basic fields for kafka form

5b45092

Various improvements to managed ingestion UI

ff20c59

Merge remote-tracking branch 'acryl/jj--fix-kafka-form' into jj--impr…

245544c

…oving-kafka-ui-ingest

github-actions bot added the product PR or Issue related to the DataHub UI/UX label Dec 1, 2022

jjoyce0510 added 2 commits November 30, 2022 16:12

Use text areas

8d40bb5

Merge remote-tracking branch 'acryl/jj--fix-kafka-form' into jj--impr…

bb142f0

…oving-kafka-ui-ingest

jjoyce0510 added 2 commits December 1, 2022 10:31

Fixing kafka form

0fd31e8

Merge remote-tracking branch 'acryl/jj--fix-kafka-form' into jj--impr…

8fdefb6

…oving-kafka-ui-ingest

chriscollins3456 approved these changes Dec 1, 2022

View reviewed changes

jjoyce0510 merged commit df96e89 into datahub-project:master Dec 1, 2022

cccs-Dustin pushed a commit to CybercentreCanada/datahub that referenced this pull request Feb 1, 2023

refactor(ui): Improving Kafka UI Ingestion Form, Create Domain, Creat…

4c41836

…e Secret Modals (datahub-project#6588)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ui): Improving Kafka UI Ingestion Form, Create Domain, Create Secret Modals #6588

refactor(ui): Improving Kafka UI Ingestion Form, Create Domain, Create Secret Modals #6588

jjoyce0510 commented Dec 1, 2022 •

edited

Loading

github-actions bot commented Dec 1, 2022 •

edited

Loading

chriscollins3456 left a comment

chriscollins3456 Dec 1, 2022

chriscollins3456 Dec 1, 2022

jjoyce0510 Dec 1, 2022

chriscollins3456 Dec 1, 2022

chriscollins3456 Dec 1, 2022

chriscollins3456 Dec 1, 2022

jjoyce0510 Dec 1, 2022

chriscollins3456 Dec 1, 2022

jjoyce0510 Dec 1, 2022

		{ min: 1, max: 50 },
		{ pattern: /^[^\s\t${}\\,'"]+$/, message: 'This secret name is not allowed.' },

refactor(ui): Improving Kafka UI Ingestion Form, Create Domain, Create Secret Modals #6588

refactor(ui): Improving Kafka UI Ingestion Form, Create Domain, Create Secret Modals #6588

Conversation

jjoyce0510 commented Dec 1, 2022 • edited Loading

Summary

Checklist

github-actions bot commented Dec 1, 2022 • edited Loading

Unit Test Results (build & test)

chriscollins3456 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjoyce0510 commented Dec 1, 2022 •

edited

Loading

github-actions bot commented Dec 1, 2022 •

edited

Loading