Feature/logger factory #2725

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

stefankleff commented Oct 10, 2012

Added the ability to pass an options array to Logger, Writer, Formatter and Filter.
Therefore it's possible to use config files to configure a logger in a factory like in ZF1.

Example:

$options = array(
  'writers' => array(
    array(
        'name' => 'stream',
        'stream' => 'php://output',
        'formatter' => 'simple',
    ),
    array(
        'name' => 'stream',
        'stream' => 'application.log',
        'formatter' => array(
            'name' => 'simple',
            'options' => array(
                'format' => '%timestamp%'
            ),
        ),
        'filters' => array(
            array(
                'name' => 'priority',
                'options'  => array(
                    'priority' => Logger::WARN
                ),
            ),
        ),
    ),
);

$logger =  new Logger($options);

No BC breaks, but I have to trigger an error instead of throwing an exception in AbstractWriter.php to keep the expected test values consistent. This should be changed but will be a (very minor) BC break.

@stefankleff stefankleff Accidentally committed one removed test. Uncommented again.
Nevertheless XmlTest::testFormatWillRemoveExtraEmptyArrayFromEvent() will fail on windows 7 x64
1c5fb48
Owner

weierophinney commented Oct 10, 2012

Since you're making this service manager friendly, could you also create a Zend\ServiceManager\FactoryInterface implementation for setting up a configured logger as well, please?

Contributor

stefankleff commented Oct 10, 2012

Sure - But can you please more concrete whats on your mind? Standalone or for use in Mvc? Mvc would be in a different namespace and using the registered "Config"-Service as well as providing an initializer. But what if no logger is used at all?

Owner

weierophinney commented Oct 10, 2012

@stefankleff Look at Zend\Db\Adapter\AdapterServiceFactory, Zend\I18n\Translator\TranslatorServiceFactory, and elsewhere for examples. Basically, you create the factory in the component namespace, and then add an entry for it in the Zend\Mvc\Service\ServiceConfig class.

Contributor

stefankleff commented Oct 12, 2012

@weierophinney I've added a factory, but didn't add an entry in the ServiceListenerFactory (I think you meant this class). This is because neither the translator nor the db-adapter is added there. Adding them will require an additional dependency in the composer.json of the mvc component. It should be done like in the skeleton app where the translator is added in the config.

@b-durand b-durand and 1 other commented on an outdated diff Oct 14, 2012

library/Zend/Log/Writer/FingersCrossed.php
{
- return $this->writer;
+ throw new Exception\InvalidArgumentException(get_class() . ' does not support formatting');
@b-durand

b-durand Oct 14, 2012

Contributor

We can't change this behavior. You should revert this change.

@stefankleff

stefankleff Oct 15, 2012

Contributor

I did this to be consistent with the MongoDB writer. If no exception should be thrown, the MongoDB writer shouldn't do this too.

@b-durand b-durand commented on an outdated diff Oct 14, 2012

library/Zend/Log/Writer/AbstractWriter.php
{
- $this->formatter = $formatter;
+ if (is_string($formatter)) {
+ $formatter = $this->formatterPlugin($formatter, $options);
+ }
+
+ if (!$formatter instanceof Formatter\FormatterInterface) {
+ // This should be used instead of triggering an error, but this will require to change tests
@b-durand

b-durand Oct 14, 2012

Contributor

Don't commit this. Put in a comment. You don't need dead code in a file.
I agree with your for this point: +1 to switch to an exception for consistency.

stefankleff added some commits Oct 15, 2012

@stefankleff stefankleff Updated behavior when setting formatter in writers which doesn't supp…
…ort formatting.

Do nothing instead of throwing an exception
66aa0e6
@stefankleff stefankleff AbstractWriter::setFormatter() throws exception if type is neither st…
…ring nor FormatterInterface.

This is a small BC break. Tests updated to check the new type of exception.
a5b5955

please return the logger too. without returning the "ServiceLocator" will throw a exception "ServiceNotCreatedException"

thanks ;)

Owner

stefankleff commented on d73ba86 Nov 8, 2012

Thank you! This is fixed in PR zendframework#2914

Contributor

coolmic commented Nov 24, 2012

Hi (sorry for my english).

As it is now, the formatter and filters are ignored, because you handle it in the construct method of AbstractWriter, but all its subclasses override it, and doesn't call the parent method.

If we want to fix it, we have to change the signature of each writer.
Don't know if it's a bc break.

@weierophinney weierophinney added a commit to zendframework/zend-log that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'feature/log-factory' into develop cf296d0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment