Skip to content

Add partition files amount limit #18

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

choseenonee
Copy link
Collaborator

Summary

This PR introduces the partition_files_limit parameter for CSV and Parquet output writers.

  • Default: 1000 files
  • Configurable: can be overridden in models.yml
  • Ignored: if the --force flag is provided

CLI changes

  • Renamed --continue-generation → --continue
  • Renamed -F → -f

Testing notes

  • The promptui package contains a data race that becomes visible when running tests with the -race flag and calling promptui.Run().
  • To avoid breaking race-enabled test runs, such tests were isolated from the rest.

@choseenonee choseenonee force-pushed the partition-amount-limit branch from 8dc7caf to ce74fd6 Compare August 12, 2025 13:54
@ReverseTM
Copy link
Collaborator

I tried to test your changes, ran the program, outputting data to csv in partitioning mode, and when the limit of 1000 files was reached, I entered ‘N’ and the program crashed with a panic.

image

@@ -0,0 +1,119 @@
package confirm
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps it would be beneficial to move confirm to the render package and generally implement it as a renderer structure method.

Comment on lines +103 to +106
type DummyReadWriteCloser struct {
io.Reader
io.Writer
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be renamed to nopReadWriteCloser to match the standard library, but that's a matter of taste.

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