Skip to content

Conversation

@szh
Copy link

@szh szh commented Feb 17, 2022

In security sensitive use cases, it would be helpful to allow the caller to know if deleting the temp file failed, so it could retry after a delay, or at least log the failure.
My main concern is how to test this functionality.

sdassow added a commit to sdassow/atomic that referenced this pull request Feb 23, 2022
@sdassow
Copy link

sdassow commented Feb 23, 2022

As I've been polishing this code I could not resist taking this into account, hence my attempt at it using option funcs above, based on earlier work in #20.
It's the least invasive way I can think of without duplicating code just for testing; if there's a cleaner approach with less effort I'd be interested to know.
At least the code to enable the testing code-path is only available during testing, so the risk should be relatively contained as well.

@szh szh marked this pull request as ready for review February 23, 2022 14:03
@sdassow
Copy link

sdassow commented Feb 24, 2022

Was actually expecting a suggestion or at least a comment with regard to testing, as that's an interesting aspect.
In respect to changing the return type, I wonder if it could stay the same by making AtomicResult comply with the error interface.
And then there still is the defer code hanging around which maybe could be removed as a consequence?

@szh
Copy link
Author

szh commented Mar 22, 2022

We ended up implementing this internally so I'm not going to be able to dedicate time to finishing up the implementation here. If you want to close the PR that's fine with me.

@szh szh closed this Aug 18, 2022
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