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

Add show_progress flag to BulkImportWriter #141

Merged
merged 6 commits into from
Dec 6, 2024

Conversation

DavidLandup0
Copy link
Contributor

Adds the show_progress flag to the BulkImportWriter, to optionally show the progress of chunking into msgpack format and uploading the temp files. Defaults to False so backwards compatibility is preserved.

Does not affect the progress of the bulk_import.perform() or bulk_import.commit() calls.

Not sure if this is even something we're looking to add so feel free to close if it's not meant to be supported (especially if we want to keep dependencies low) - I just found it personally to be a nice touch for developer experience when working with large DataFrame uploads :)

@DavidLandup0 DavidLandup0 requested review from tung-vu-td and chezou and removed request for tung-vu-td December 5, 2024 07:54
@@ -1,10 +1,10 @@
"""IPython Magics

IPython magics to access to Treasure Data. Load the magics first of all:
IPython magics to access to Treasure Data. Load the magics first of all:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure why the linter decided to remove the tab here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the linter is correct though. Multiline comments usually align with the """, not the text

Copy link
Member

@chezou chezou left a comment

Choose a reason for hiding this comment

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

Left comments. Other than that looks good.

pytd/writer.py Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
Copy link
Contributor

@tung-vu-td tung-vu-td left a comment

Choose a reason for hiding this comment

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

LGTM

@DavidLandup0 DavidLandup0 merged commit 74abe50 into master Dec 6, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants