Skip to content

Modify ingest script to check a local directory#443

Merged
naglepuff merged 2 commits intomainfrom
local-vetting-ingest
Apr 3, 2026
Merged

Modify ingest script to check a local directory#443
naglepuff merged 2 commits intomainfrom
local-vetting-ingest

Conversation

@naglepuff
Copy link
Copy Markdown
Collaborator

No description provided.

@naglepuff naglepuff force-pushed the local-vetting-ingest branch 2 times, most recently from 87be481 to 14e2115 Compare April 2, 2026 16:30
@naglepuff naglepuff marked this pull request as ready for review April 2, 2026 16:30
@naglepuff naglepuff requested a review from BryonLewis April 2, 2026 16:30
Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

Minor comment about file being open to check for existence.

assert data_dir
filename = str(data_dir / s3_key)
try:
f = open(filename)
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.

should this just be a Path(dat_dir /s3_key) exists?
Otherwise I think you're opening a file in text mode, leaving it open and then later on opening it in binary mode when you create the recording.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, fixed now

@naglepuff naglepuff force-pushed the local-vetting-ingest branch from 14e2115 to f4f57f4 Compare April 2, 2026 21:49
@BryonLewis BryonLewis self-requested a review April 2, 2026 23:18
@BryonLewis
Copy link
Copy Markdown
Collaborator

I'm also noticing linting is failing if you want to resolve that first

@naglepuff naglepuff merged commit b8bd9dc into main Apr 3, 2026
4 checks passed
@naglepuff naglepuff deleted the local-vetting-ingest branch April 3, 2026 21:29
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.

2 participants