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

Ensure h5file is closed even if validation fails #52

Merged
merged 1 commit into from
May 19, 2024

Conversation

ramenbytes
Copy link
Contributor

@ramenbytes ramenbytes commented Mar 26, 2021

This fixes a bug where if validation of the Photon-HDF5 file failed, then save_photon_hdf5() would not close it even if the user had requested us to do so. This behavior created an awkward situation if the user did not save the file object in a variable somewhere, since they would be unable to close it and thus unable to try saving to it again without restarting the python process. With this fix, save_photon_hdf5() becomes more amenable to experimenting at the console without interrupting the flow of things to restart the process after mistakenly trying to save malformed data.

Callers of the save_photon_hdf5() can specify that they wanted the hdf5
file they passed in closed before returning. However, we performed
validation before closing the file, and the validation can error out. If
this happened, the file /would not be closed/. Not a bit deal if the
user still had a hold on the file object and could close it themselves,
but if they instead passed in just the file name then they were unable
to close it themselves without somehow getting a hold of the object from
python's global list of open files. This meant that they would need to
restart the python process before they could try saving to the hdf5 file
again.

Now, we place the file closing logic in the clause of a finally:
statement to ensure we always close it if the user wants us to.
@ramenbytes
Copy link
Contributor Author

I looked at the CI failures, and it doesn't appear to be related to this pull request. Something about missing DLLs I think.

@ramenbytes
Copy link
Contributor Author

Ping

@harripd harripd merged commit 28488e6 into Photon-HDF5:master May 19, 2024
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