Skip to content

Memoize SNS client #91

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
Merged
rapsad merged 3 commits into from
Sep 10, 2025
Merged

Memoize SNS client #91

rapsad merged 3 commits into from
Sep 10, 2025

Conversation

@andrzejbisewski
Copy link
Contributor

@andrzejbisewski andrzejbisewski commented Aug 29, 2025 •
edited
Loading

For sending a message:

3.4.3 :006 > 2.times {   TestPublisher.new.call({ event_data: "some value" }) }
getting backend..
getting backend..
getting backend..
getting backend..

the backend was called multiple times - each time initialising a new SNS client

Its used in ahplus-rails https://github.com/AirHelp/ahplus-rails/blob/master/Gemfile#L98

@andrzejbisewski andrzejbisewski self-assigned this Aug 29, 2025
@andrzejbisewski andrzejbisewski marked this pull request as ready for review August 29, 2025 11:28
seban
seban previously approved these changes Aug 29, 2025
View reviewed changes
cinkonaap
cinkonaap previously approved these changes Aug 29, 2025
View reviewed changes
@andrzejbisewski andrzejbisewski dismissed stale reviews from cinkonaap and seban via 71c3029 September 9, 2025 10:46
rapsad
rapsad previously approved these changes Sep 9, 2025
View reviewed changes
przprz
przprz previously approved these changes Sep 10, 2025
View reviewed changes
przprz
przprz reviewed Sep 10, 2025
View reviewed changes
lib/eventboss/sns_client.rb Outdated
Comment on lines 42 to 43
@backend ||= begin
if configured?
Copy link
Contributor

@przprz przprz Sep 10, 2025

Choose a reason for hiding this comment

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

Suggested change
@backend ||= begin
if configured?
@backend ||=
begin
if configured?

🙏🏻

@andrzejbisewski andrzejbisewski dismissed stale reviews from przprz and rapsad via 4b56346 September 10, 2025 13:26
@andrzejbisewski
Copy link
Contributor Author

andrzejbisewski commented Sep 10, 2025

Can you please squash merge?

@rapsad rapsad merged commit cd1a5c0 into master Sep 10, 2025
7 checks passed
@rapsad rapsad deleted the cache-client branch September 10, 2025 16:13
@rapsad rapsad restored the cache-client branch September 10, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@przprz przprz przprz left review comments

@rapsad rapsad rapsad approved these changes

@seban seban seban left review comments

@cinkonaap cinkonaap cinkonaap left review comments

Assignees

@andrzejbisewski andrzejbisewski

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@andrzejbisewski @seban @przprz @rapsad @cinkonaap