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

IVA Notifications (GSI-741) #5

Merged
merged 15 commits into from
May 8, 2024
Merged

Conversation

TheByronHimes
Copy link
Member

Adds notifications according to the sugar ant epic regarding IVA state changes.
When consuming the configured event type, the key is checked to see whether it begins with 'all-'. If so, the orchestrator's "process_all_ivas_invalidated" method is called. Otherwise, "process_iva_state_change" is called.

The process_iva_state_change method will double-check the value of the state field (only within the context of the notifications).
If all checks out, it will the call a private method corresponding to the type of notification to send.

@TheByronHimes TheByronHimes requested a review from Cito May 6, 2024 13:29
Copy link
Member

@Cito Cito left a comment

Choose a reason for hiding this comment

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

Well done. Just a couple of small suggestions.

src/nos/core/notifications.py Outdated Show resolved Hide resolved
src/nos/core/notifications.py Outdated Show resolved Hide resolved
src/nos/core/notifications.py Outdated Show resolved Hide resolved
src/nos/core/notifications.py Show resolved Hide resolved
src/nos/core/notifications.py Show resolved Hide resolved
src/nos/ports/outbound/dao.py Show resolved Hide resolved
src/nos/translators/inbound/event_sub.py Show resolved Hide resolved
tests/test_orchestrator.py Show resolved Hide resolved
tests/test_orchestrator.py Outdated Show resolved Hide resolved
tests/test_orchestrator.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented May 7, 2024

Pull Request Test Coverage Report for Build 8997560664

Details

  • 60 of 69 (86.96%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.9%) to 88.889%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/nos/ports/inbound/orchestrator.py 7 9 77.78%
src/nos/core/orchestrator.py 28 35 80.0%
Totals Coverage Status
Change from base Build 8896670124: -0.9%
Covered Lines: 256
Relevant Lines: 288

💛 - Coveralls

@TheByronHimes TheByronHimes requested a review from Cito May 7, 2024 08:34
Copy link
Member

@Cito Cito left a comment

Choose a reason for hiding this comment

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

Looks good. Two rewording suggestions, feel free to ignore if you think yours is better.

src/nos/core/notifications.py Show resolved Hide resolved
src/nos/core/notifications.py Outdated Show resolved Hide resolved
tests/test_orchestrator.py Outdated Show resolved Hide resolved
tests/test_orchestrator.py Show resolved Hide resolved
TheByronHimes and others added 3 commits May 8, 2024 08:45
Co-authored-by: Christoph Zwerschke <cito@online.de>
Co-authored-by: Christoph Zwerschke <cito@online.de>
@TheByronHimes TheByronHimes merged commit 0196c9d into main May 8, 2024
8 checks passed
@TheByronHimes TheByronHimes deleted the feature/iva_notifications_GSI-741 branch May 8, 2024 07:01
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.

3 participants