Skip to content

feat(ingestion-ui) Display CLI-based ingestion sources in UI#5681

Merged
jjoyce0510 merged 4 commits intodatahub-project:masterfrom
chriscollins3456:cc--display-cli-ingestion-sources
Aug 19, 2022
Merged

feat(ingestion-ui) Display CLI-based ingestion sources in UI#5681
jjoyce0510 merged 4 commits intodatahub-project:masterfrom
chriscollins3456:cc--display-cli-ingestion-sources

Conversation

@chriscollins3456
Copy link
Copy Markdown
Collaborator

@chriscollins3456 chriscollins3456 commented Aug 18, 2022

Adds necessary checks and styles for displaying CLI based ingestion sources in the managed ingestion list in the UI. If it's from CLI, disable the execute button, only allow them to view their recipe (not edit), and have a few small styling changes.

The VAST majority of the diff here is from refactoring. If you view the changes by commit, this will be much easier to review.

Some screenshots:
one snowflake CLI ingestion source
image

when you expand its executions
image

When you click the "View" button for CLI sources - read-only viewer of the recipe
image

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 18, 2022

Unit Test Results (build & test)

504 tests  ±0   504 ✔️ ±0   8m 33s ⏱️ -11s
117 suites ±0       0 💤 ±0 
117 files   ±0       0 ±0 

Results for commit 7d603fe8. ± Comparison against base commit ad57fb9.

♻️ This comment has been updated with latest results.

@maggiehays maggiehays added the product PR or Issue related to the DataHub UI/UX label Aug 18, 2022
return (
(source === 'MANUAL_INGESTION_SOURCE' && 'Manual Execution') ||
(source === 'SCHEDULED_INGESTION_SOURCE' && 'Scheduled Execution') ||
(source === MANUAL_INGESTION_SOURCE && 'Manual Execution') ||
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

indentSize: 0,
}}
pagination={false}
<IngestionSourceTable
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much cleaner- thank you!

cliIngestion: source.config.executorId === CLI_EXECUTOR_ID,
}));

return (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful!

}
`;

export function TypeColumn(type: string, record: any) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should do this for all of our tables. Really like splitting columns out like this

Copy link
Copy Markdown
Collaborator

@jjoyce0510 jjoyce0510 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a model PR. You solved the problem AND made the code much easier to read & maintain. Huge!

@jjoyce0510 jjoyce0510 merged commit e189be7 into datahub-project:master Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

product PR or Issue related to the DataHub UI/UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants