Skip to content
This repository has been archived by the owner on Feb 6, 2022. It is now read-only.

Flush spool queue for console terminate as well #64

Merged
merged 2 commits into from
Nov 4, 2013
Merged

Flush spool queue for console terminate as well #64

merged 2 commits into from
Nov 4, 2013

Conversation

BitOne
Copy link
Contributor

@BitOne BitOne commented Oct 23, 2013

Mail spool were only flush at the end of a HTTP Request (KernelEvents:TERMINATE). In case of a console command sending mail using a spool, the spool was not flushed, so no mail was sent (and in case of MemorySpool, the mail were lost).

So this PR subscribes to the ConsoleEvents:TERMINATE to do the same flushing as in the context of a HTTP request.

@@ -33,7 +34,7 @@ public function __construct(ContainerInterface $container)
$this->container = $container;
}

public function onKernelTerminate(PostResponseEvent $event)
public function onTerminate($event)
Copy link
Member

Choose a reason for hiding this comment

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

can you remove the argument entirely as it is not used anyway ?

@gquemener
Copy link

👍

@BitOne
Copy link
Contributor Author

BitOne commented Nov 4, 2013

Hello guys,

Is there a problem with this PR ?

Can you tell me what is missing or wrong so I can try to fix it so it can be merged in the master ? Right, now the workaround is quite ugly as it's a copy and paste of the onTerminate function inside the command.

Thanks a lot !

fabpot added a commit that referenced this pull request Nov 4, 2013
This PR was merged into the master branch.

Discussion
----------

Flush spool queue for console terminate as well

Mail spool were only flush at the end of a HTTP Request (KernelEvents:TERMINATE). In case of a console command sending mail using a spool, the spool was not flushed, so no mail was sent (and in case of MemorySpool, the mail were lost).

So this PR subscribes to the ConsoleEvents:TERMINATE to do the same flushing as in the context of a HTTP request.

Commits
-------

4de0f63 Cleanup use and unused parameter
4b90392 Flush spool queue for console terminate as well
1c87809 Merge pull request #61 from ruian/patch-1
336d332 Remove redondant parameter
@fabpot fabpot merged commit 4de0f63 into symfony:master Nov 4, 2013
@BitOne
Copy link
Contributor Author

BitOne commented Nov 4, 2013

Thanks !

@BitOne BitOne deleted the fix/console_terminate branch November 4, 2013 11:27
@stof
Copy link
Member

stof commented Nov 4, 2013

@0s1r1s you are confusing the SwiftmailerBundle version and the symfony version. They don't need to stay in sync all the time (the master branch is currently compatible with all symfony versions as of 2.1).

But the change will indeed require bumping the dependency (and we should bump the version to 2.3 for this)

@0s1r1s
Copy link

0s1r1s commented Nov 4, 2013

I just thought that if someone will install the 2.2 branch of symfony-standard like it is it will directly fail because the version 2.2 of Swiftmailer is default in the composer.json
https://github.com/symfony/symfony-standard/blob/2.2/composer.json

And perhaps I'm a little bit sleepy but if you say that it's compatible with all symfony versions as of 2.1 it should be compatible with this composer.json I sent, shouldn't it?

@stof
Copy link
Member

stof commented Nov 4, 2013

@0s1r1s The composer.json indeed need to be changed. what I was referring to when saying that you were confusing both versions was your first paragraph. The SwiftmailerBundle version and the Console component version are different. They have separate release cycles. So nothing forbids SwiftmailerBundle 2.2 to use class introduced in Console 2.3, assuming it defines its requirements properly (which is not the case right now as this PR would require changing requirements).
So you are right sayign there is an issue on the composer.json, but you were wrong when explaining what was the issue. It is not related to the bundle alias but to the bundle requirement.

As we will restrict the supported Symfony version, we will need to bump the bundle version to 2.3. But even if we were bumping the minimal version to Symfony 2.4, the next version of the bundle would still be 2.3 (after 2.2, we go to 2.3, not 2.4).

@0s1r1s
Copy link

0s1r1s commented Nov 4, 2013

@stof okay that totally makes sense. That was just my language barrier ;-) Thx for your explanation

@pborreli
Copy link
Contributor

pborreli commented Nov 5, 2013

having same issue using symfony/symfony-standard 2.2

@stof
Copy link
Member

stof commented Nov 12, 2013

fixed now

BitOne added a commit to akeneo/BatchBundle that referenced this pull request Feb 12, 2014
weaverryan added a commit to symfony/symfony-docs that referenced this pull request Aug 13, 2014
…pool in the CLI (stof)

This PR was merged into the 2.3 branch.

Discussion
----------

Added a note about the automatic handling of the memory spool in the CLI

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets | n/a

since symfony/swiftmailer-bundle#64 flushing the memory spool manually is not necessary anymore when using Symfony 2.3+. I added a versionadded note here, but we might decide to remove the obsolete paragraph instead, making the cookbook only about generating urls (which would require renaming the page then)

Commits
-------

c99c70d Added a note about the automatic handling of the memory spool in the CLI
z38 pushed a commit to oro-subtree/BatchBundle that referenced this pull request Jul 14, 2016
inri13666 pushed a commit to oro-flex/batch-bundle that referenced this pull request Dec 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants