-
-
Notifications
You must be signed in to change notification settings - Fork 539
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
[5.x] Add ability to specify the queue connection on static:warm command #8634
[5.x] Add ability to specify the queue connection on static:warm command #8634
Conversation
Thank you. I made the tests run. The comment said |
Yeah I had the same experience ¯_(ツ)_/¯ |
I want to be sure that we make the difference between the queue connection and queue itself very obvious. i.e. When you do I'm not sure which is the right option. Both could be argued, so we should find some other examples somewhere and follow convention. |
Yeah I kind of had the same question. I did it this way since there's the When looking at the Maybe make the |
…cms into static-warm-queue-connection
Hey @jasonvarga I made some improvements to hopefully make it more clear to differentiate between queue name and connection. Let me know, thanks. |
(I'll also fix the tests, just wondering if you think it's all right) |
Hey @jasonvarga do you mind revisiting this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be working well for me. Just had two thoughts around the new config option.
config/static_caching.php
Outdated
| | ||
*/ | ||
|
||
'queue_connection' => env('STATAMIC_STATIC_WARM_QUEUE_CONNECTION'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it might be worth changing this to warm_queue_connection
to stay consistent with warm_queue
above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I wonder if it might be worth falling back to the default connection here so we don't need to do that in the StaticWarm
command:
'queue_connection' => env('STATAMIC_STATIC_WARM_QUEUE_CONNECTION'), | |
'warm_queue_connection' => env('STATAMIC_STATIC_WARM_QUEUE_CONNECTION', config('queue.default')), |
# Conflicts: # config/static_caching.php # src/Console/Commands/StaticWarm.php # tests/Console/Commands/StaticWarmTest.php
…helpers pass the test again.
…it'll figure out which connection based on your config.
I've updated this so that If you use |
* Allow configuring the Stache's Cache Store Related: statamic/cms#10303 * Ability to disable CP authentication Related: statamic/cms#8960 * Display custom logo as plain text Related: statamic/cms#10350 * Track sites.yaml path in git integration config Related: statamic/cms#10463 * Add ability to specify the queue connection on static:warm command Related: statamic/cms#8634
This is adding another workaround for #3291. It seems like the default queue connection needs to be
sync
, but when I deploy changes I'd like to warm using the queue. This change allows you to specify the connection using the--queue
option.I added an additional test, even though they are marked as incomplete at the moment. I also updated another assertion to match the current output. In my experience the tests seems ok.
Closes statamic/ideas#1153.