Skip to content

Comments

refactor(deletion): isolate steps to remove blocks#7332

Closed
jfagoagas wants to merge 1 commit intomasterfrom
improve-deletion-process
Closed

refactor(deletion): isolate steps to remove blocks#7332
jfagoagas wants to merge 1 commit intomasterfrom
improve-deletion-process

Conversation

@jfagoagas
Copy link
Member

Context

Please include relevant motivation and context for this PR.

If fixes an issue please add it with Fix #XXXX

Description

Please include a summary of the change and which issue is fixed. List any dependencies that are required for this change.

Checklist

API

  • Verify if API specs need to be regenerated.
  • Check if version updates are required (e.g., specs, Poetry, etc.).
  • Ensure new entries are added to CHANGELOG.md, if applicable.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Mar 20, 2025

Codecov Report

Attention: Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.

Project coverage is 92.17%. Comparing base (07b9e1d) to head (01fd759).
Report is 481 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7332      +/-   ##
==========================================
- Coverage   92.25%   92.17%   -0.09%     
==========================================
  Files          81       81              
  Lines        7374     7375       +1     
==========================================
- Hits         6803     6798       -5     
- Misses        571      577       +6     
Flag Coverage Δ
api 92.17% <64.70%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
prowler ∅ <ø> (∅)
api 92.17% <64.70%> (-0.09%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@drewkerrigan drewkerrigan left a comment

Choose a reason for hiding this comment

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

Worth a try - the risk I guess is some orphaned data, but it should be easy to find if that happens.

deletion_summary.update(step_summary)
except DatabaseError as error:
logger.error(f"Error deleting {step_name}: {error}")
raise
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise
raise error

Copy link
Member

Choose a reason for hiding this comment

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

Not needed. Python will raise the captured exception by default.

deletion_summary.update(provider_summary)
except DatabaseError as error:
logger.error(f"Error deleting Provider: {error}")
raise
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise
raise error

Copy link
Member

Choose a reason for hiding this comment

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

Not needed. Python will raise the captured exception by default.

@jfagoagas
Copy link
Member Author

Closing this since it is implemented in #7349

@jfagoagas jfagoagas closed this Mar 24, 2025
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.

3 participants