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

Add ScopedPostgresResource #188

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add ScopedPostgresResource #188

wants to merge 6 commits into from

Conversation

Tishj
Copy link
Collaborator

@Tishj Tishj commented Sep 18, 2024

This PR feels like a continuation of #176 and is also related to #93

When we throw a C++ exception we have to make sure that all postgres related resources get released, this is done through the destructor of the ScopedPostgresResource class

Opening as a draft, hopefully we can talk about the implementation or even alternatives that solve the same issue

…e can release the resource with the destructor
@Tishj Tishj requested a review from JelteF September 18, 2024 12:51
@JelteF
Copy link
Collaborator

JelteF commented Sep 19, 2024

I think this goal of this PR makes a lot of sense, but I think this needs a review from someone that's more experienced in C++ than me. @Y-- what do you think about this? Are there easier/better ways to achieve the same?

@Y--
Copy link
Collaborator

Y-- commented Sep 19, 2024

I think this goal of this PR makes a lot of sense, but I think this needs a review from someone that's more experienced in C++ than me. @Y-- what do you think about this? Are there easier/better ways to achieve the same?

This LGTM, a pretty typical use of RAII pattern :-)

@Tishj
Copy link
Collaborator Author

Tishj commented Sep 20, 2024

Thanks, then I'll add it to more places as required and undraft 👍

@Tishj Tishj marked this pull request as ready for review September 20, 2024 12:28
@JelteF JelteF changed the title [Dev] Add ScopedPostgresResource Add ScopedPostgresResource Sep 25, 2024
@JelteF JelteF added this to the 0.1.0 code complete milestone Sep 30, 2024
@JelteF JelteF added the enhancement New feature or request label Sep 30, 2024
@JelteF JelteF modified the milestones: 0.1.0 code complete, 0.2.0 Oct 15, 2024
@JelteF
Copy link
Collaborator

JelteF commented Oct 15, 2024

Moved this to 0.2.0, in that release we'll do a more holistic approach to solving these issues.

@JelteF
Copy link
Collaborator

JelteF commented Nov 6, 2024

@Y-- Does this still make sense? Or should we close this one? It seems like you haven't needed this for your cleanup so far?

@Y--
Copy link
Collaborator

Y-- commented Nov 6, 2024

@Y-- Does this still make sense? Or should we close this one? It seems like you haven't needed this for your cleanup so far?

Yeah I think something like that still make sense. I'll drive it through or re-implement a similar version when I come to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants