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

[Bifrost] Decouple loglet errors from bifrost errors #1705

Merged
merged 2 commits into from
Jul 23, 2024
Merged

Conversation

AhmedSoliman
Copy link
Contributor

@AhmedSoliman AhmedSoliman commented Jul 8, 2024

[Bifrost] Decouple loglet errors from bifrost errors

  • Major cleanup of loglet error handling, this removes the unnecessary leakage of internal loglet error types to bifrost.
  • No more superflous Arc<> of non-Clone errors.
  • FindTail on loglets cannot return Sealed error (enforcement via type-system)

Stack created with Sapling. Best reviewed with ReviewStack.

Copy link

github-actions bot commented Jul 8, 2024

Test Results

102 files  ±0  102 suites  ±0   22m 57s ⏱️ -3s
 84 tests ±0   84 ✅ ±0  0 💤 ±0  0 ❌ ±0 
217 runs  +2  217 ✅ +2  0 💤 ±0  0 ❌ ±0 

Results for commit 88a94eb. ± Comparison against base commit bf9f0d1.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

Nice separation of the LogletError :-) LGTM. +1 for merging.


/// Represents a type-erased error from the loglet provider.
pub trait LogletError: std::error::Error + Send + Sync + Debug + Display + 'static {
/// Signal upper layers whether this erorr should be retried or not.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/erorr/error/

@AhmedSoliman AhmedSoliman force-pushed the pr1705 branch 3 times, most recently from 7e8dc57 to 0d078d3 Compare July 19, 2024 13:35
A restructure the allows finer-grain control over what types loglet providers have access to. This will be exploited more in upcoming PRs.
- Major cleanup of loglet error handling, this removes the unnecessary leakage of internal loglet error types to bifrost.
- No more superflous `Arc<>` of non-Clone errors.
- FindTail on loglets cannot return Sealed error (enforcement via type-system)
@AhmedSoliman AhmedSoliman merged commit 88a94eb into main Jul 23, 2024
9 checks passed
@AhmedSoliman AhmedSoliman deleted the pr1705 branch July 23, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants