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.

Sign up for GitHub

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

Jump to bottom

Treatment service Poller #87

Merged
merged 20 commits into from
Jan 2, 2025
Merged

Treatment service Poller #87

merged 20 commits into from
Jan 2, 2025

Conversation

rautelaanirudh
Copy link
Contributor

What this PR does / why we need it:
The intention of this PR is to provide the option of utilising a poller in the treatment service instead of pub/sub. The poller will be responsible for pulling the changes periodically instead of subscribing to the management service topic.

Poller can be enabled from treatment service config

PollerConfig:
  Enabled: true
  PollInterval: 10s

Which issue(s) this PR fixes:

Treatment service can be used in cloud environments where pub/sub is currently not supported e.g. Tencent cloud.

Anirudh Rautela and others added 16 commits December 15, 2024 18:17
Added changes for using queue only if poller is not enabled in config
fdaa24a
Added changes for poller config in treatment service
70c1407
Added changes for poller in treatment service
2b9fd44
Added changes for poller in treatment service
46f6ecb
Added changes for poller in treatment service
eb29b76
Reverted poller changes for management-service
e7b91a3
Fixed treatment service config test case
7c8c331
Refactored poller
7760cbd
Fixed poller condition
a7ee54e
Refactored poller
814fc9d
Removed unused struct
fa9f4d5
Integrated PR review comments
0e953f2
Integrated PR review comments
63bd2c6
Integrated PR review comments
c79dde1
@rautelaanirudh
Merge pull request #1 from rautelaanirudh/MM3-2915_treatment_service_…
e5fe78b 
…poller

Merging changes for [MM3 2915] Treatment service Poller
Added sample poller config
9a5357c
deadlycoconuts
deadlycoconuts requested changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

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

Thanks a lot of this PR! There are just a couple of small comments I left, especially on the polling logic which I didn't really understand why it needed to be implemented as such.

treatment-service/appcontext/appcontext.go Outdated Show resolved Hide resolved
treatment-service/server/server.go Outdated Show resolved Hide resolved
treatment-service/config/config.go Outdated Show resolved Hide resolved
treatment-service/server/poller.go Outdated Show resolved Hide resolved
treatment-service/server/poller.go Outdated Show resolved Hide resolved
treatment-service/server/poller.go Outdated Show resolved Hide resolved
deadlycoconuts
deadlycoconuts approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

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

Thanks a lot for all the changes! I've just one last tiny comment but feel free to merge this once that's addressed! :D 🚀

@deadlycoconuts deadlycoconuts merged commit 397e59b into caraml-dev:main Jan 2, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deadlycoconuts deadlycoconuts deadlycoconuts approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rautelaanirudh @deadlycoconuts