refactor(ui): Making improvements to UI ingestion forms, adding MySQL, Trino, Presto, MSSQL, MariaDB forms #6607
Conversation
| rules: null, | ||
| required: true, |
There was a problem hiding this comment.
The ingestion may work even without specifying a warehouse if user has a DEFAULT_WAREHOUSE set (see https://docs.snowflake.com/en/sql-reference/sql/alter-user.html) and if the configured role has USAGE access on that warehouse. However I agree that specifying warehouse explicitly is recommended for various reasons(troubleshooting, clarity).
There was a problem hiding this comment.
Yeah, I'd rather keep as required for now, even though technically you can get aware with it..
| rules: null, | ||
| required: true, |
There was a problem hiding this comment.
Snowflake ingestion supports multiple authentication types including default (username/password) auth, key-pair, oauth. This is driven by authentication_type config in recipe. The "required" nature of username is therefore conditional and depends on authentication_type.
There was a problem hiding this comment.
Currently we only support username / password thru the UI, so until we support Key-Pair as well, I think we should leave as required
| section: 'Views', | ||
| setValueOnRecipeOverride: (recipe: any, values: string[]) => | ||
| setListValuesOnRecipe(recipe, values, viewDenyFieldPath), | ||
| required: true, |
There was a problem hiding this comment.
Same comments as warehouse.
There was a problem hiding this comment.
Okay - will leave for now to reduce the likelihood of ingestion failure
datahub-web-react/src/app/ingest/source/builder/RecipeForm/common.tsx
Outdated
Show resolved
Hide resolved
datahub-web-react/src/app/ingest/source/builder/RecipeForm/common.tsx
Outdated
Show resolved
Hide resolved
datahub-web-react/src/app/ingest/source/builder/RecipeForm/hive.ts
Outdated
Show resolved
Hide resolved
datahub-web-react/src/app/ingest/source/builder/RecipeForm/common.tsx
Outdated
Show resolved
Hide resolved
jjoyce0510
changed the title
datahub-web-react/src/app/ingest/source/builder/RecipeForm/RecipeForm.tsx
Outdated
Show resolved
Hide resolved
| getValueFromRecipeOverride: (recipe: any) => { | ||
| return get(recipe, isProfilingEnabled); | ||
| }, | ||
| setValueOnRecipeOverride: (recipe: any, value: boolean) => { | ||
| return setFieldValueOnRecipe(recipe, value, isProfilingEnabled); | ||
| }, |
There was a problem hiding this comment.
I don't think setting these override methods are necessary since isProfilingEnabled is set as the fieldPath.
There was a problem hiding this comment.
oh nice catch yes this is simple!
datahub-web-react/src/app/ingest/source/builder/RecipeForm/common.tsx
Outdated
Show resolved
Hide resolved
…, Trino, Presto, MSSQL, MariaDB forms (datahub-project#6607)
Summary
In this PR, we add a new form to UI ingestion for
We also refactor every other form for copy edits, required field status, and more.
Screenshots
Checklist