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.

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

Do not open more than one migration or on-disk representation file at a time #173

Merged
merged 15 commits into from
Feb 21, 2024

Conversation

mzabani
Copy link
Owner

@mzabani mzabani commented Feb 21, 2024

Fixes issue #172 and adds a test to assert this never happens again

This doesn't come even close to fixing the issue, but felt like a good
place to start.
This still doesn't solve the issue, but will likely play an important
role, as code elsewhere won't have to worry about explicitly closing
files when they're done with them.

This shouldn't be a problem because we can't rewind file streams and
because `hClose` can be called more than once on the same handle.
This is not a pretty implementation.
Env vars to be templated are stored in each `SqlMigration` object (both
their names and values), and one of the functions that return migrations
and streams from disk higher up in the internal layers of the codebase
now reads that, closes the stream it had before and opens a new one.

That new FileStream is returned by calling the parser again and overriding
specific parts of the returned record, assuming the file won't change
in between the two moments it is opened (I don't think we can escape
this assumption regardless of implementation if we want to parse+report
errors in migrations before we start applying them).
And cleanup argv handling in the Runfile
Because they will obviously fail without strace-wrapping,
and it's not worth implementing it for them.
@mzabani mzabani force-pushed the issue-172-open-only-one-migration-at-a-time branch from aeec6eb to 622244a Compare February 21, 2024 21:50
@mzabani mzabani merged commit ef02bc9 into master Feb 21, 2024
2 checks passed
@mzabani mzabani deleted the issue-172-open-only-one-migration-at-a-time branch February 21, 2024 22:09
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.

1 participant