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

Thread safety for FastaDir file handle cache #117

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Thread safety for FastaDir file handle cache #117

merged 2 commits into from
Nov 13, 2023

Conversation

theferrit32
Copy link
Contributor

@theferrit32 theferrit32 commented Aug 28, 2023 •
edited
Loading

Addresses #112

Also:

  • Changed a test to use whatever tempfile determined to be the tmp file location, which can vary by OS and can be overridden.
  • Made the seqrepo-rest-service endpoint used in tests configurable, like the SEQREPO_ROOT_DIR is. Useful if the port or path needs to be different on a particular machine (though this does make the resulting cassette values differ and those should not be committed to git)

This was referenced Aug 29, 2023
Closed
Merged
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale Issue is stale and subject to automatic closing label Oct 6, 2023
@theferrit32 theferrit32 removed the stale Issue is stale and subject to automatic closing label Oct 6, 2023
@theferrit32 theferrit32 mentioned this pull request Oct 6, 2023
Closed
@theferrit32
Copy link
Contributor Author

theferrit32 commented Oct 6, 2023 •
edited
Loading

Oh no I broke something

EDIT: force pushed a cherry-picked branch to clean it up

@theferrit32
Add explicit close to FastaFiles and sqlite3 cursors. Add contextmana…
7a85d24 
…ger locking around cached open FabgzReaders
@theferrit32
Make tests pass. Don't assume /tmp is the OS tmpfile path.
701f0b3 
Make seqrepo-rest-service endpoint used in tests configurable with SEQREPO_REST_URL

Add comment to _open_for_reading. Add close to alias db handle
@theferrit32 theferrit32 mentioned this pull request Oct 6, 2023
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 6, 2023

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale Issue is stale and subject to automatic closing label Nov 6, 2023
@theferrit32 theferrit32 removed the stale Issue is stale and subject to automatic closing label Nov 8, 2023
@reece reece merged commit 48958d7 into biocommons:main Nov 13, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reece reece reece approved these changes

Assignees
No one assigned
Labels
None yet
Projects
biocommons near-term roadmap
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@theferrit32 @reece