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 eventengine plugin for juniper alert count #2519

Merged

Conversation

johannaengland
Copy link
Contributor

@johannaengland johannaengland commented Nov 28, 2022

Closes #2432

@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Merging #2519 (6639689) into master (e2b92de) will decrease coverage by 0.03%.
The diff coverage is 92.42%.

❗ Current head 6639689 differs from pull request most recent head 74fc9d1. Consider uploading reports for the commit 74fc9d1 to get more accurate results

@@            Coverage Diff             @@
##           master    #2519      +/-   ##
==========================================
- Coverage   54.74%   54.71%   -0.03%     
==========================================
  Files         560      561       +1     
  Lines       40732    40778      +46     
==========================================
+ Hits        22298    22311      +13     
- Misses      18434    18467      +33     
Files Changed Coverage Δ
python/nav/eventengine/__init__.py 70.58% <68.75%> (-29.42%) ⬇️
...ython/nav/eventengine/plugins/juniperalertcount.py 100.00% <100.00%> (ø)

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Nov 28, 2022

Test results

     12 files       12 suites   13m 50s ⏱️
3 185 tests 3 185 ✔️ 0 💤 0
9 030 runs  9 030 ✔️ 0 💤 0

Results for commit 74fc9d1.

♻️ This comment has been updated with latest results.

@johannaengland johannaengland force-pushed the eventengine/juniper-alert-count branch 2 times, most recently from 44d6783 to 8ae69cf Compare January 5, 2023 07:30
@johannaengland johannaengland force-pushed the eventengine/juniper-alert-count branch from a1fafbe to 02091c9 Compare June 1, 2023 11:36
Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

sigh codecov is so chatty it's hard to say what's actually going on here. More tests please, just to shut it up. Unless there is a way to hide all the codecov stuff. Anyone? @lunkwill42? Bueller?

@lunkwill42
Copy link
Member

sigh codecov is so chatty it's hard to say what's actually going on here. More tests please, just to shut it up. Unless there is a way to hide all the codecov stuff. Anyone? @lunkwill42? Bueller?

More tests are always welcome. As mentioned elsewhere, annotations from CodeCov can be hidden on a file-by-file basis from the "triple dot" menu at the top right of each file header, @hmpf

@johannaengland johannaengland force-pushed the eventengine/juniper-alert-count branch from 172ccc7 to ad14ebb Compare August 4, 2023 14:02
@Uninett Uninett deleted a comment from sonarcloud bot Aug 4, 2023
@Uninett Uninett deleted a comment from sonarcloud bot Aug 4, 2023
@Uninett Uninett deleted a comment from sonarcloud bot Aug 4, 2023
Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Maybe too aggressive logging.

Some questions, which if answered will make it easier to understand what's going on once the code is merged and forgotten. (Without having to understand/remember what eventengine etc. does)

python/nav/eventengine/plugins/juniperalertcount.py Outdated Show resolved Hide resolved
python/nav/eventengine/plugins/juniperalertcount.py Outdated Show resolved Hide resolved
python/nav/eventengine/plugins/juniperalertcount.py Outdated Show resolved Hide resolved
Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

This looks like it will pretty much do the trick. Just nitpicks on cleanliness from my side 👍

python/nav/eventengine/plugins/juniperalertcount.py Outdated Show resolved Hide resolved
tests/integration/eventengine/juniper_alert_count_test.py Outdated Show resolved Hide resolved
Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

👍

@sonarcloud
Copy link

sonarcloud bot commented Sep 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johannaengland johannaengland merged commit fb4ea26 into Uninett:master Sep 5, 2023
13 checks passed
@johannaengland johannaengland deleted the eventengine/juniper-alert-count branch September 5, 2023 06:27
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.

Write new eventengine plugin to handle Juniper chassis/system alert count transitions
3 participants