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 closeOnExcept to clean up code that closes resources only on exceptions #456

Merged
merged 1 commit into from
Jul 29, 2020

Conversation

jlowe
Copy link
Member

@jlowe jlowe commented Jul 28, 2020

I've been bumping into a number of cases where a resource is obtained, some computation needs to happen on that resource, then ownership of the resource is handed off to somewhere else. During the computation it's important to cover the exception path, which means writing code that looks something like this:

val r = ...some code that obtains a resource...
try {
  ...some code that computes on the resource...
} catch {
  case t: Throwable =>
    r.safeClose()
    throw t
}

The withResource code is a nice wrapper for handling the case where a resource must always be closed, but we don't have something to cover the case where a resource should only be closed if an exception occurs.

This PR adds closeOnExcept methods that are similar to withResource but closing in a catch clause rather than a finally block. It also updates code that was using this code pattern.

@jlowe jlowe added the SQL part of the SQL/Dataframe plugin label Jul 28, 2020
@jlowe jlowe added this to the Jul 20 - Jul 31 milestone Jul 28, 2020
@jlowe jlowe self-assigned this Jul 28, 2020
abellina
abellina previously approved these changes Jul 28, 2020
@jlowe
Copy link
Member Author

jlowe commented Jul 28, 2020

build

Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look fine but could you please sign your commit like we are asking others to do.

@jlowe
Copy link
Member Author

jlowe commented Jul 29, 2020

build

@jlowe
Copy link
Member Author

jlowe commented Jul 29, 2020

We need the fix from #461 or to turn off the noscaladoc rule before the CI build is going to pass.

@abellina
Copy link
Collaborator

build

@jlowe jlowe merged commit 42d0564 into NVIDIA:branch-0.2 Jul 29, 2020
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@jlowe jlowe deleted the close-on-except branch September 10, 2021 15:30
pxLi pushed a commit to pxLi/spark-rapids that referenced this pull request May 12, 2022
* update FLAdminAPI runner

* fix typo
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
Signed-off-by: spark-rapids automation <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SQL part of the SQL/Dataframe plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants