Skip to content
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(ingest): add ingest --no-progress option #9300

Merged
merged 9 commits into from
Dec 14, 2023

Conversation

BlueHorn07
Copy link
Contributor

@BlueHorn07 BlueHorn07 commented Nov 24, 2023

Checklist

Hello, guys. I want to silent intermediate ingestion report.

When, I ingest data from Databricks or Redshift platform, it takes more than 30 minutes, and it generates large log files (ex. 11 Mb). I want to avoid this situation, as it slows down our logging platform.

I added --quiet, -q option on ingest-cli, before the --dry-run, but please let me know that it's order should be changed.

  • 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 github-actions bot added ingestion PR or Issue related to the ingestion of metadata docs Issues and Improvements to docs labels Nov 24, 2023
@BlueHorn07 BlueHorn07 changed the title Make Ingest CLI Quite Make Ingest CLI Slient Nov 24, 2023
@BlueHorn07 BlueHorn07 changed the title Make Ingest CLI Slient Make Ingest CLI Silent Nov 24, 2023
Copy link
Collaborator

@hsheth2 hsheth2 left a comment

Choose a reason for hiding this comment

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

I agree with the overall premise here - those messages can be annoyingly chatty at times

My main worry is the flag's naming. In most CLIs, --quiet suppresses all log messages and is effectively the inverse of --verbose.

Can we rename quiet to --no-progress or --no-print-progress to more accurately reflect what it does?

@BlueHorn07
Copy link
Contributor Author

Hello, @hsheth2, thanks for your comment :) I agree that your worry about expression --quiet. I think it's nice to replace the expression to --no-progress. I will change it, thanks!!

Copy link
Collaborator

@hsheth2 hsheth2 left a comment

Choose a reason for hiding this comment

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

LGTM

@hsheth2 hsheth2 changed the title Make Ingest CLI Silent feat(ingest): add ingest --no-progress option Nov 29, 2023
@hsheth2 hsheth2 added the merge-pending-ci A PR that has passed review and should be merged once CI is green. label Nov 29, 2023
@BlueHorn07
Copy link
Contributor Author

Thank you for taking care of this PR, @hsheth2. Please, schedule the PR can be merged 🙏

@anshbansal anshbansal merged commit 70e64e8 into datahub-project:master Dec 14, 2023
53 checks passed
Salman-Apptware pushed a commit to Salman-Apptware/datahub that referenced this pull request Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Issues and Improvements to docs ingestion PR or Issue related to the ingestion of metadata merge-pending-ci A PR that has passed review and should be merged once CI is green.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants