Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Start to abstract message brokers #1746

Merged
merged 11 commits into from
Feb 9, 2024
Merged

Conversation

rkm
Copy link
Member

@rkm rkm commented Feb 8, 2024

Proposed Changes

Start to refactor RabbitMqAdapter logic into generic interface

  • Rename IRabbitMqAdapter -> IMessageBroker
  • Move into Smi.Common.Messaging namespace
  • Add MessageBrokerType and MessageBrokerFactory (not currently used)
  • Create ConnectionFactory directly in RabbitMQBroker

This will be part of a larger refactor of the messaging logic.

Types of changes

What types of changes does your code introduce? Tick all that apply.

  • Bugfix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation-Only Update (if none of the other choices apply)
    • In this case, ensure that the message of the head commit from the source branch is prefixed with [skip ci]

Checklist

By opening this PR, I confirm that I have:

  • Reviewed the contributing guidelines for this repository
  • Ensured that the PR branch is in sync with the target branch (i.e. it is automatically merge-able)
  • Updated any relevant API documentation
  • Created or updated any tests if relevant
  • Created a news file
    • NOTE: This must include any changes to any of the following files: default.yaml, any of the RabbitMQ server configurations, GlobalOptions.cs
  • Listed myself in the CONTRIBUTORS file 🚀
  • Requested a review by one of the repository maintainers

Issues

Copy link

codecov bot commented Feb 8, 2024

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (09067d3) 59.63% compared to head (a1288b2) 59.68%.

Files Patch % Lines
...Processor/Execution/DicomDirectoryProcessorHost.cs 0.00% 4 Missing ⚠️
...rc/common/Smi.Common/Execution/MicroserviceHost.cs 60.00% 4 Missing ⚠️
...s.CohortExtractor/Execution/CohortExtractorHost.cs 0.00% 3 Missing ⚠️
src/applications/Setup/EnvironmentProbe.cs 0.00% 1 Missing ⚠️
src/common/Smi.Common/Messaging/RabbitMQBroker.cs 92.30% 0 Missing and 1 partial ⚠️
...ionalMapper/Execution/DicomRelationalMapperHost.cs 0.00% 1 Missing ⚠️
...DicomReprocessor/Execution/DicomReprocessorHost.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1746      +/-   ##
==========================================
+ Coverage   59.63%   59.68%   +0.04%     
==========================================
  Files         185      184       -1     
  Lines        6672     6675       +3     
  Branches      933      932       -1     
==========================================
+ Hits         3979     3984       +5     
+ Misses       2426     2425       -1     
+ Partials      267      266       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rkm rkm changed the title Feature/abstract message broker Start to abstract message brokers Feb 8, 2024
@rkm rkm marked this pull request as ready for review February 8, 2024 16:12
@rkm rkm requested a review from a team February 8, 2024 16:13
@jas88 jas88 merged commit aec0b0d into main Feb 9, 2024
23 checks passed
@jas88 jas88 deleted the feature/abstract-message-broker branch February 9, 2024 18:10
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants