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.

Sign up for GitHub

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

Jump to bottom

Parameterize exception type caught by failure_callback_resource_adaptor #898

Merged
merged 7 commits into from
Nov 9, 2021
Merged

Parameterize exception type caught by failure_callback_resource_adaptor #898

merged 7 commits into from
Nov 9, 2021

Conversation

harrism
Copy link
Member

@harrism harrism commented Oct 27, 2021 •
edited
Loading

#892 added failure_callback_resource_adaptor which provides the ability to respond to memory allocation failures. However, it was hard-coded to catch (and rethrow) std::bad_alloc exceptions. This PR makes the type of exception the adaptor catches a template parameter, to provide greater flexibility. The default exception type is now rmm::out_of_memory since we expect this to be the common use case.

Also a few changes to fix clang-tidy warnings.

@github-actions github-actions bot added the cpp Pertains to C++ code label Oct 27, 2021
@harrism harrism added the breaking Breaking change label Oct 27, 2021
@harrism harrism added this to PR-WIP in v21.12 Release via automation Oct 27, 2021
@harrism harrism added the improvement Improvement / enhancement to an existing function label Oct 27, 2021
@harrism harrism self-assigned this Oct 27, 2021
@harrism harrism marked this pull request as ready for review October 27, 2021 00:47
@harrism harrism requested a review from a team as a code owner October 27, 2021 00:47
v21.12 Release automation moved this from PR-WIP to PR-Reviewer approved Oct 27, 2021
madsbk
madsbk approved these changes Oct 27, 2021
View reviewed changes
Copy link
Member

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @harrism

@ajschmidt8
Copy link
Member

rerun tests

1 similar comment
@harrism
Copy link
Member Author

harrism commented Nov 9, 2021

rerun tests

@harrism
Copy link
Member Author

harrism commented Nov 9, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 728a117 into rapidsai:branch-21.12 Nov 9, 2021
v21.12 Release automation moved this from PR-Reviewer approved to Done Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rongou rongou rongou approved these changes

@madsbk madsbk madsbk approved these changes

@jrhemstad jrhemstad jrhemstad approved these changes

Assignees

@harrism harrism

Labels
breaking Breaking change cpp Pertains to C++ code improvement Improvement / enhancement to an existing function
Projects
No open projects
v21.12 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@harrism @ajschmidt8 @rongou @madsbk @jrhemstad