Skip to content

add errors for file size - #918

Merged
mccalluc merged 6 commits into
mainfrom
901-warn-large-csv
Jul 13, 2026
Merged

mccalluc merged 6 commits into
mainfrom
901-warn-large-csv

Conversation

@mccalluc

@mccalluc mccalluc commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

@github-project-automation github-project-automation Bot moved this to Pending in DP Wizard Mar 12, 2026
@mccalluc
mccalluc marked this pull request as draft March 12, 2026 18:13
@cla-bot cla-bot Bot added the cla-signed label Jul 6, 2026
@mccalluc
mccalluc requested review from eddiestudies and removed request for eddiestudies July 6, 2026 15:04
@mccalluc
mccalluc requested a review from eddiestudies July 7, 2026 16:39
@mccalluc
mccalluc marked this pull request as ready for review July 7, 2026 16:40
Comment thread tests/utils/test_csv_helper.py Outdated
reported size: 1048576
blocks used: 0
"""
f = path.open("ab")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

with path.open("ab") as f:
f.truncate(size_in_mb * 1024 * 1024)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally prefer to use with blocks here. They protect against more error cases

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It will call the close in the end so that line could be removed

def test_get_csv_names_mismatch():
with tempfile.TemporaryDirectory() as tmp:
with TemporaryDirectory() as tmp:
a_path = Path(tmp) / "a.csv"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have written path.join so many times, this syntax is a lot better

... print('blocks used:', path.stat().st_blocks)
reported size: 1048576
blocks used: 0
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm after the with block here

@mccalluc
mccalluc requested a review from eddiestudies July 13, 2026 19:32

@eddiestudies eddiestudies left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@mccalluc
mccalluc merged commit eb91d59 into main Jul 13, 2026
5 checks passed
@mccalluc
mccalluc deleted the 901-warn-large-csv branch July 13, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Pending

Development

Successfully merging this pull request may close these issues.

Profile large CSVs... and warn on upload

2 participants