Skip to content

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Oct 14, 2020

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #38558
License MIT
Doc PR Should be added

With inspiration from the CacheComponent. This PR makes it possible to make the connection to Redis only when you first use it.

@natepage
Copy link
Contributor

@Nyholm Thank you for the quick reply + patch!

- MESSENGER_NOTIFICATIONS_TRANSPORT_DSN=redis://redis:6379/messages
+ MESSENGER_NOTIFICATIONS_TRANSPORT_DSN=redis://redis:6379/messages?lazy=1

Working like a charm!

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(with one comment to fix before merging)

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

👍

fabpot added a commit that referenced this pull request Oct 14, 2020
This PR was merged into the 4.4 branch.

Discussion
----------

[Cache] Make Redis initializers static

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes .. or maybe?
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

I am on very thin ice now. I saw a comment on similar code here: #38563 (comment)

These anonymous functions in the cache component could also be made static to avoid being connected to the object using the Redis trait.

Feel free to correct me if this does not make much sense.

Commits
-------

ad8de57 [Cache] Make Redis initializers static
@fabpot fabpot force-pushed the messenger-redis-lazy-connect branch from 378557a to 1d7c801 Compare October 14, 2020 14:11
@fabpot
Copy link
Member

fabpot commented Oct 14, 2020

Thank you @Nyholm.

@Nyholm
Copy link
Member Author

Nyholm commented Oct 14, 2020

Thank you for the reviews, tests and merge.

@Nyholm Nyholm deleted the messenger-redis-lazy-connect branch October 14, 2020 14:23
@fabpot fabpot mentioned this pull request Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Messenger] Automatic schema updates with redis transport requires redis to run during database migrations
6 participants