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.

Sign up for GitHub

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

Jump to bottom

Fix data dump script regression #10426

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Fix data dump script regression #10426

merged 1 commit into from
Feb 5, 2025

Conversation

jimchamp
Copy link
Collaborator

@jimchamp jimchamp commented Feb 5, 2025

Closes #9521

Reverts recent changes to the xopen function, which were causing a closed file object to be returned in some cases.

Technical

Testing

Screenshot

Stakeholders

@github-actions github-actions bot added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Feb 5, 2025
tfmorris
tfmorris approved these changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@tfmorris tfmorris left a comment

Choose a reason for hiding this comment

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

Those original changes were weirdly asymmetric and never would have worked.

cdrini
cdrini approved these changes Feb 5, 2025
View reviewed changes
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Darn! Good catch; seems like there was a miscommunication on the previous pr, this line wasn't meant to be merged.

@cdrini cdrini merged commit 0b85ee0 into internetarchive:master Feb 5, 2025
4 checks passed
@cdrini cdrini assigned cdrini and unassigned mekarpeles Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tfmorris tfmorris tfmorris approved these changes

@cdrini cdrini cdrini approved these changes

Assignees

@cdrini cdrini

Labels
Priority: 1 Do this week, receiving emails, time sensitive, . [managed]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Data Dumps not auto-generated
4 participants
@jimchamp @tfmorris @cdrini @mekarpeles