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

Deprecate SkipException to help avoid confusion #3662

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Deprecate SkipException to help avoid confusion #3662

merged 1 commit into from
Feb 2, 2023

Conversation

ryan-blakley
Copy link
Contributor

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

Mark SkipException deprecated to help avoid confusion as the dirty parser difference isn't really needed. Replaced all internal references with SkipComponent.

@ryan-blakley ryan-blakley requested a review from xiangce January 24, 2023 18:49
docs/exception_model.rst Outdated Show resolved Hide resolved
docs/exception_model.rst Outdated Show resolved Hide resolved
insights/core/exceptions.py Outdated Show resolved Hide resolved
insights/core/exceptions.py Outdated Show resolved Hide resolved
@xiangce
Copy link
Contributor

xiangce commented Jan 31, 2023

BTW, this PR breaks the tests of gss and prodsec rules, I raised a MR to resolve the test of gss rules and asked ProdSec team to resolve the prodsec rules as well.

* Mark SkipException deprecated to help avoid confusion as the dirty
  parser difference isn't really needed.
* Replaced all internal references with SkipComponent.

Signed-off-by: Ryan Blakley <[email protected]>
@ryan-blakley
Copy link
Contributor Author

@xiangce Another good catch I tested the other projects but forgot about gss-rules. In addition to your PR, I corrected the issue by adding a back import in insights/core/init.py for SkipException. All tests pass for me now on all projects that I know about.

@jholecek-rh
Copy link
Contributor

@ryan-blakley Thank you. The PR looks good. Our rule repository won't be affected. We haven't been using SkipException exceptions for over a year now. All tests pass.

@xiangce xiangce merged commit d46b4eb into RedHatInsights:master Feb 2, 2023
xiangce pushed a commit that referenced this pull request Feb 2, 2023
* Mark SkipException deprecated to help avoid confusion as the dirty
  parser difference isn't really needed.
* Replaced all internal references with SkipComponent.

Signed-off-by: Ryan Blakley <[email protected]>
(cherry picked from commit d46b4eb)
xiangce pushed a commit that referenced this pull request Sep 6, 2024
* Mark SkipException deprecated to help avoid confusion as the dirty
  parser difference isn't really needed.
* Replaced all internal references with SkipComponent.

Signed-off-by: Ryan Blakley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants