Skip to content
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

[MRG+1] Python logging #1060

Merged
merged 21 commits into from Apr 29, 2015
Merged

[MRG+1] Python logging #1060

merged 21 commits into from Apr 29, 2015

Conversation

@curita
Copy link
Member

@curita curita commented Mar 2, 2015

This PR switches Twisted logging system to the Python standard one.

It's pretty much done, though it's missing documentation (UPDATE: All docs related to logging have been updated) since I want to hear feedback first. I implemented the bare minimum changes to support all previous use cases, but new functionality could definitely be added.

The pull request closes a few open issues and other PRs. This closes #921, closes #881, closes #877, closes #875, closes #483, closes #481, closes #433, and closes #265.

@curita curita force-pushed the curita:python-logging branch from d4af954 to 10fec41 Mar 2, 2015
@@ -7,7 +10,9 @@
from scrapy.utils.conf import arglist_to_dict
from scrapy.utils.spider import iterate_spider_output, spidercls_for_request
from scrapy.exceptions import UsageError
from scrapy import log

logger = logging.getLogger('scrapy')

This comment has been minimized.

@kmike

kmike Mar 2, 2015
Member

What do you think about using logging.getLogger(__name__)? This way users will be able to adjust logging output of individual modules and subpackages. Stdlib logging supports loggers hierarchy: if you add a formatter and set a log level for 'scrapy' logger, this formatter and log level will be used for 'scrapy.commands.parse' as well, unless you override it.

This comment has been minimized.

@curita

curita Mar 3, 2015
Author Member

I like the idea. Actually I used scrapy logger for a kind of weak reason, I wanted to maintain the [scrapy] prefix on the logging format and that's the easiest way (just need to provide a [%(name)s] on the logformatter). I think I prefer to name each logger individually (for example, I want to have a scrapy.requests logger to mimic Django's django.requests) but it's probably better to use __name__ as default on each module and treat those cases separately.

This comment has been minimized.

@kmike

kmike Mar 3, 2015
Member

I don't quite like [scrapy.requests] because there is no scrapy.requests module/package; it could make things harder to debug - it is harder to find where the logs came from. But I'm not sure, maybe __name__ in log messages is too long. Could you please paste some examples after you try it?

This comment has been minimized.

@curita

curita Mar 3, 2015
Author Member

It's a matter of formatting, you can have the path from where the log call was issued with the %(pathname)s placeholder, regardless of logger's name.

This comment has been minimized.

@kmike

kmike Mar 3, 2015
Member

aha, this makes sense.

This comment has been minimized.

@curita

curita Mar 4, 2015
Author Member

Here's an example of the output using __name__ loggers: https://gist.github.com/Curita/00ddfbddc28cd6f11934. I don't think there are many longer paths than scrapy.contrib.downloadermiddleware.redirect on Scrapy.

This comment has been minimized.

@kmike

kmike Mar 11, 2015
Member

We can also use a custom Formatter which uses [scrapy] for all our loggers, stripping the hierarchy, but keeps other names untouched. Or a default format string which just uses [scrapy] regardless of logger name - this formatter can be attached only to scrapy logger, not to user logs.

This comment has been minimized.

@curita

curita Mar 19, 2015
Author Member

Added here: 69af4f7

@@ -123,22 +127,21 @@ def __init__(self, settings):
super(CrawlerProcess, self).__init__(settings)
install_shutdown_handlers(self._signal_shutdown)
self.stopping = False
self.log_observer = log.start_from_settings(self.settings)
log.scrapy_info(settings)
configure_logging(settings)

This comment has been minimized.

@kmike

kmike Mar 2, 2015
Member

Python docs suggest not to add logger handlers by default in libraries. Maybe we can move this from Crawler to Scrapy command-line module?

This comment has been minimized.

@curita

curita Mar 3, 2015
Author Member

I agree, but CrawlerProcess is intended to be a ScrapyCommand util, which makes Scrapy behave as a framework (it starts the reactor too). On #873 we're going to move this class to another place and this distinction would be hopefully more obvious. I plan to document that using configure_logging without settings won't create any handlers, for custom script purposes.

warnings.warn("Module `scrapy.log` has been deprecated, Scrapy relies now on "
"the builtin Python library for logging. Read the updated "
"logging entry in the documentation to learn more.",
ScrapyDeprecationWarning, stacklevel=2)

This comment has been minimized.

@kmike

kmike Mar 2, 2015
Member

I think it worths it to add log.msg and log.err aliases for backwards compatibility.

This comment has been minimized.

@curita

curita Mar 3, 2015
Author Member

Absolutely, I'll add both.

This comment has been minimized.

@kmike

kmike Mar 3, 2015
Member

I've also seen log.DEBUG / log.INFO /... constants in use.
It probably doesn't make sense to keep aliases for log.start and alike methods though.

@curita curita changed the title [WIP] Python logging Python logging Mar 10, 2015

If you plan on configuring the handlers yourself is still recommended you
call this function, keeping `settings` as ``None``. Bear in mind there
won't be any log output set by default in that case.

This comment has been minimized.

@kmike

kmike Mar 11, 2015
Member

It could be good to explain how to get the output in this case.

This comment has been minimized.

@curita

curita Mar 19, 2015
Author Member

Added it in cf1d4a4.

@curita curita force-pushed the curita:python-logging branch 2 times, most recently from 1dd9f5e to 66f735d Mar 19, 2015
@curita
Copy link
Member Author

@curita curita commented Mar 19, 2015

I rebased scrapy:master and changed the docs with the new log output.

warnings.warn("Module `scrapy.log` has been deprecated, Scrapy relies now on "
"the builtin Python library for logging. Read the updated "
"logging entry in the documentation to learn more.",
ScrapyDeprecationWarning, stacklevel=2)

This comment has been minimized.

@berkerpeksag

berkerpeksag Mar 19, 2015
Contributor

I think stacklevel=2 can be removed since the warning.warn() call is in the module level.

This comment has been minimized.

@nyov

nyov Mar 25, 2015
Contributor

nitpick: "Scrapy relies now on" -> "Scrapy now relies on"

This comment has been minimized.

@curita

curita Mar 30, 2015
Author Member

I think stacklevel=2 can be removed since the warning.warn() call is in the module level.

@berkerpeksag We're using that stacklevel so the warning shows the line where scrapy.log was imported, instead of the line where the warning is issued in scrapy.log.

This comment has been minimized.

@curita

curita Mar 30, 2015
Author Member

nitpick: "Scrapy relies now on" -> "Scrapy now relies on"

@nyov Thanks! I rebased it in 61b5d20

This comment has been minimized.

@berkerpeksag

berkerpeksag Mar 30, 2015
Contributor

Oh, I didn't think about that use case. Good point. Thanks for the explanation.

@@ -1,8 +1,12 @@
import json
import socket
import logging

from testfixtures import LogCapture

This comment has been minimized.

@berkerpeksag

berkerpeksag Mar 19, 2015
Contributor

There is now a assertLogs helper in unittest module: https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertLogs We've backported it to Django's django.test.TestCase (not merged yet) to use in earlier Python versions. Perhaps instead of adding testfixtures dependency, Scrapy can backport it too. That way, Scrapy users can test(of course they will need to update their tests to use e.g. scrapy.test.TestCase instead of unittest.TestCase) their programs output without rely on testfixtures or another dependency.

This comment has been minimized.

@kmike

kmike Mar 20, 2015
Member

That's an interesting idea, but I'd prefer a dependency. Django maintains a backport of assertLogs, unittest2 maintains a backport of assertLogs, it'd be too much copy-paste if Scrapy start to do the same. Using unittest2 dependency instead of testfixtures sounds good to me - the code becomes more compatible with 3.x standard library. But it could be harder from technical point of view: Scrapy uses twisted.trial.unittest.TestCase which inherits from 'unittest.TestCase', not from 'unittest2.TestCase'.

This comment has been minimized.

@curita

curita Mar 30, 2015
Author Member

I like unittest2 too. Problem right now is that some TestCases can't be changed to inherit directly from unittest2.TestCase (or even unittest.TestCase) instead of twisted.trial.unittest.TestCase because the twisted one implements the logic to wait for deferreds, so some tests that use defer.inlineCallbacks are going to return immediately after switching those parent classes (and possibly be marked as successful tests without checking anything).

I just realized that after verifying that the plugin pytest-twisted won't work on test methods from unittest.TestCase objects (that plugin implements the logic to wait for deferreds), since pytest handles separately its integration with unittest. I'd probably need to submit another PR checking that unittest.TestCase is used only when not dealing with deferreds, so I prefer that this PR uses testfixtures package for now.

@nyov
Copy link
Contributor

@nyov nyov commented Mar 25, 2015

This is just massive. I dare not comment on the changes lest I stick my foot in it.

But I do like the old name conventions better.
So is there any chance to keep or change the general naming convention self.log.level() instead of self.logger.level() in spider classes? And log.level() instead of logger.level() for others?

And I liked the brevity of having the loglevel as an optional function argument instead of a requirement in the name. But oh well.

filename = settings.get('LOG_FILE')
if filename:
encoding = settings.get('LOG_ENCODING')
handler = logging.FileHandler(filename, encoding=encoding)

This comment has been minimized.

@kmike

kmike Mar 30, 2015
Member

Where is handler.close() called for FileHandler?

Not calling it may cause issues like scrapinghub/scrapyrt#9.

This comment has been minimized.

@curita

curita Apr 1, 2015
Author Member

Where is handler.close() called for FileHandler?

Not calling it may cause issues like scrapinghub/scrapyrt#9.

Since we're only setting a single top level FileHandler Scrapy shouldn't run out of file descriptors (at least for this cause). Since this a handler for the root logger it should be closed at the very end, for example we could call logging.shutdown() in CrawlerProcess._signal_shutdown and CrawlerProcess._signal_kill but Scrapy is stopping either way at this point, file descriptors are going to be released on their own.

This comment has been minimized.

@chekunkov

chekunkov Apr 7, 2015
Contributor

@curita I personally vote for closing file handler explicitly - it's best practice. Leaving file handler opened and relying on the fact that it will be closed on interpreter exit make code buggy and hard to reuse in other projects. For example scrapinghub/scrapyrt#14 wouldn't have existed if someone had thought about this while implementing current scrapy logging

@kmike
Copy link
Member

@kmike kmike commented Mar 30, 2015

Is it posisble to route logs from each spider/Crawler to a separate file when several spiders are executed in parallel?

Scrapyrt does that. no quite: as @chekunkov said, if many spider logs files are opened in the same process twisted log observer cannot filter those messages and they appear in all opened log files

@chekunkov
Copy link
Contributor

@chekunkov chekunkov commented Mar 30, 2015

if many spider logs files are opened in the same process twisted log observer cannot filter those messages and they appear in all opened log files

This doesn't apply to all log messages, rather to messages without crawler object in event dict. So messages sent from spider like self.log('msg') are going to be sent to correct destination file and wouldn't appear in other files. Basically because of this line. But if you use log.msg() - like it's usually done in middlewares, pipelines and many places in scrapy core - log observer wouldn't filter this message and it will appear in all log files. While this is not a big problems when you have single crawl per process, this causing troubles when multiple crawls are opened in a single process, and yes, this problem wasn't solved in ScrapyRT and I hope we can find some way to fix this behaviour during migration to python logging.

What could possibly fix this is prohibiting log.msg() method and forcing everyone to use some kind of crawler.log() method everywhere - idea is to have reference to crawler object in every event dict - then messages related to different crawls can be 100% reliably sent to destinations without showing up in other places.

All above applies only to twisted logging. I haven't checked this PR yet - possibly it solves this issue? I'll try to read it carefully tomorrow.

@curita
Copy link
Member Author

@curita curita commented Mar 30, 2015

@nyov:

So is there any chance to keep or change the general naming convention self.log.level() instead of self.logger.level() in spider classes? And log.level() instead of logger.level() for others?

I prefer self.logger since the actual object is a Python logger, and you have a log method in there, calling self.log.log("level", "msg") is kind of confusing. Name "logger" at the top of the modules, like:

logger = logging.getLogger(__name__)

is merely a convention, but it's a well established one.

And I liked the brevity of having the loglevel as an optional function argument instead of a requirement in the name. But oh well.

There's a logger.log("level", "msg") function available in Python loggers, and self.log in the Spider is still there (it calls to self.logger.log now).

@nyov
Copy link
Contributor

@nyov nyov commented Mar 31, 2015

and self.log in the Spider is still there

Ah thanks, I missed that. All is well again. And you're right, convention is good, 'log.log' would be ugly :) I will endure.

@curita
Copy link
Member Author

@curita curita commented Mar 31, 2015

@kmike, @chekunkov:
Oh, I think this is going to raise some doubts, routing all spider/crawler messages into a single logger (thus not supporting directly the redirection to different files in a per-spider basis) was a design decision, I'll try to explain why I think this is a better approach.

First of all, Scrapy needs to log things before crawlers are instantiated. It also needs to log messages independently entirely from crawlers, for example, if LOG_STDOUT is set print messages will be redirected to the logging system, and those clearly won’t have a Crawler attached to them. The same goes for messages logged outside Scrapy by other libraries, like Twisted.

On top of that, there are messages logged inside the crawler when you haven’t a spider instance yet (all extension initialization happens before the spider is instantiated) so there are messages outside spiders too.

Because of that there are always going to be messages outside crawlers. By itself I think this is a deal breaker, we can’t link every messages to a crawler or spider, and it’s not clear what to do with those unlinked (which crawler is going to configure them? they are going to be present in crawler outputs, facing duplication?) and I think any decision here will be somehow arbitrary.

Also, we’re switching to python logging, so discouraging the users to use it directly, forcing them to call Scrapy wrappers over it (like with a crawler.log() call) goes against one of the main ideas for this change.

Another problem is that there’s no easy way around passing Crawler or Spider instances to the extra parameter in log messages, and then filtering them with custom logging.Filters per crawler/spider to get those messages issued by a specific crawler. This makes it hard to use the available tools for configuring logging with the standard lib, so we would discard a lot of benefits for this port. For example, logging.Handlers (which take care of redirecting log messages to different sources) can’t be attached to filters, only loggers, logging.Formatters only attach to handlers, we can’t attach filters to other filters (we’re using filters in scrapy.utils.log), etc.

Those issues could be bypassed by using different loggers per Crawler/Spider, but there isn't a convenient way to implement this. Loggers are referenced by their name, so we should have a unique string representation for every instance. I know that the Spider.name is the first thing that comes to mind, but this is just not robust enough. It could change at any time during the Spider execution. It could be defined in its init call, not as class attribute, so we won’t be able to log anything until then. And more importantly, we can actually run two spiders at the same time with the same name, so we won’t be able to differentiate their loggers. I think we could use a hash or id of the instance as logger name, but this will make the log output non-deterministic, and loggers will be hard to reference manually.

Messages attached to crawlers are still given an instance of them in their extra parameters so implementing something that redirects messages in a per-crawler basis is still possible, but I rather keep it outside Scrapy to maintain simplicity and consistency.

method to send log messages from your spider
@property
def logger(self):
logger = logging.getLogger(self.name)

This comment has been minimized.

@chekunkov

chekunkov Apr 2, 2015
Contributor

how log messages will be propagated if logger is not a child of 'scrapy' logger?

This comment has been minimized.

@curita

curita Apr 10, 2015
Author Member

how log messages will be propagated if logger is not a child of 'scrapy' logger?

This messages aren't propagated on purpose, since they are issued by the user and not Scrapy. Since this is actually a helper for creating a logger and logging through it directly, and we can't control what loggers the users create, I think it makes more sense to detach it from Scrapy logging.

This comment has been minimized.

@chekunkov

chekunkov Apr 14, 2015
Contributor

this will break logging for existing spiders, for example a lot of user that are using dash currently will experience unexpected disappearance of spider logs (including quite important WARNING and ERROR level logs) and to fix that they all would need to setup log handlers. Don't you think that by default it would be better to provide logger that would be handled by scrapy and only if user refuses to use default logger and creates new one (which is advanced usage which will be available only after merging this PR) - then he will take all responsibility for creating appropriate handlers?

It's also not clear for me how dash itself would handle spider logs after this change. @krotkiewicz / @dangra are you ready for this?

This comment has been minimized.

@krotkiewicz

krotkiewicz Apr 14, 2015

Dash is fine here, so question is to @dangra.

This comment has been minimized.

@curita

curita Apr 14, 2015
Author Member

@chekunkov It shouldn't break existing spiders while running locally, all log options are applied to the root logger so they're going to be propagated to all loggers, not just scrapy descendants.

Code in hworkers has to adapt to support all these changes (remove the manual twisted logging handling), and it should support using any python loggers too. I'm not sure about this but I think it already supports python logging, but it automatically sets its level to INFO: https://github.com/scrapinghub/hworker/blob/master/hworker/bot/log.py#L40. Still, we should see how to handle this in scrapinghub/hworker when this PR gets accepted I think.

This comment has been minimized.

@chekunkov

chekunkov Apr 15, 2015
Contributor

all log options are applied to the root logger so they're going to be propagated to all loggers, not just scrapy descendants

ok, sounds good, I missed that

This comment has been minimized.

@dangra

dangra Apr 21, 2015
Member

hworker mostly works but there are some issues with backwards compatibility of log.msg(), see https://gist.github.com/dangra/ee2287a099680aade28a#file-log_loremipsum_2-txt-L23.

This comment has been minimized.

@kmike

kmike Apr 22, 2015
Member

I like scrapy.core.scraper or scrapy.contrib.logstats names in @dangra's gist. It helps with debugging to see where the log (error) message came from.

But doesn't it mean TopLevelFormatter is not working?

This comment has been minimized.

@curita

curita Apr 22, 2015
Author Member

@dangra: sorry about that, I didn't test the backward compatibility functions too much. I've fixed it and rebased master, can you test it again?

@kmike: hworker sets the logging configuration itself I think, without calling configure_logging, that should be the reason why TopLevelFormatter isn't set. I like logging the whole logger name too, but I'd make that change after the module relocation so we don't have to update the log samples in the docs twice.

@chekunkov
Copy link
Contributor

@chekunkov chekunkov commented Apr 2, 2015

@curita

As far as Scrapy is used to run one spider per process that's not a problem. But unfortunately Scrapy is used in other projects, like ScrapyRT, and by design ScrapyRT runs all crawls in one process. That wasn't my decision, it was discussed and agreed with @shaneaevans, reason for this is performance considerations. So, when you have multiple crawlers in a single process and logging is not designed provide you a way to filter messages related only to one particular crawl - messages from all crawls running at the same time would mix which makes logging unusable.

First of all, Scrapy needs to log things before crawlers are instantiated

Can you provide an example? I think you mean logging that's coming from commands code. In my opinion it's perfectly fine that those log messages doesn't have crawler object in extra as it wasn't instantiated yet and there's no need to associate such messages with any crawler.

if LOG_STDOUT is set print messages will be redirected to the logging system, and those clearly won’t have a Crawler attached to them. The same goes for messages logged outside Scrapy by other libraries, like Twisted.

that's not a big deal as well, at least in context of ScrapyRT such messages will be filtered and no one will miss them if they wouldn't appear in crawl log files.

Loggers are referenced by their name, so we should have a unique string representation for every instance. I know that the Spider.name is the first thing that comes to mind, but this is just not robust enough. It could change at any time during the Spider execution. It could be defined in its init call, not as class attribute, so we won’t be able to log anything until then. And more importantly, we can actually run two spiders at the same time with the same name, so we won’t be able to differentiate their loggers.

In case of ScrapyRT one crawler creates exactly one spider like it expected by latest Scrapy version - crawler doesn't support having multiple Spider objects any more. So no, I'm not talking about one logger per spider object, I'm talking about one logger per crawler object. And what I expect from Scrapy logging is to be able to create filter which will take messages related only to one crawler object and then send them to log file (or use in any other way). All messages coming from one spider should be handled by crawler logger. Also crawler contains references to engine, scraper, downloader, middlewares, pipelines, extensions objects (as far as I know we don't have singletones there, different crawlers reference different objects) - all messages coming from those objects should also be handled by crawler logger.

Regards possible name conventions and implementation - unfortunately have no brilliant ideas. Crawl logger should be descendant of 'scrapy', so it could be 'scrapy.crawler_{object_id}'. You said:

but this will make the log output non-deterministic, and loggers will be hard to reference manually.

To deal with this crawler object may have method which will return proper logger object and this way of obtaining logger should be used in all pipelines, middlewares, pipelines, extensions, engine, scraper, schdeduler, downloader etc. And it's not clear what you mean by 'non-deterministic' - if loggers with have the same prefix that shouldn't cause problems.

Also, we’re switching to python logging, so discouraging the users to use it directly, forcing them to call Scrapy wrappers over it (like with a crawler.log() call) goes against one of the main ideas for this change.

You say we want users to use it directly - so why Spider.logger was added? This goes against direct usage of logger, as direct usage would be defining logger like logger = logging.getLogger(__name__) at the top of spider module or during __init__.

Switching to python logging has much more other benefits then ability to get logger by using logger = logging.getLogger(__name__) - loggers hierarchy, configuration flexibility, all these facilities like Handlers, Filters, Formatters etc.

To summarize - I tried to show why proposed logging design is not perfect and how it will break depending projects. If we merge it like it is now - I have no good ideas how to support it in ScrapyRT. Bad ideas:

  1. log everything in a single file - I think this have no sense
  2. log messages if they have reference to crawler of spider object - logging will miss a lot of useful log messages
  3. disable logging completely - yeah, really who needs those log messages :)
  4. return to the implementation where each crawl is started in separate process <- @shaneaevans ?
@chekunkov
Copy link
Contributor

@chekunkov chekunkov commented Apr 2, 2015

I should correct myself

So no, I'm not talking about one logger per spider object, I'm talking about one logger per crawler object.
...
Crawl logger should be descendant of 'scrapy', so it could be 'scrapy.crawler_{object_id}'
...

actually creating separate logger per crawler is not required, I don't know why I concentrated only on this solution, using 'scrapy' logger with crawler object in extra parameter should be enough as well. The main idea is not to have separate logger per crawler but to be able to split log messages coming from different crawlers and related objects. Please consider this while reading my previous comment

@curita
Copy link
Member Author

@curita curita commented Apr 10, 2015

Thanks @chekunkov for reviewing this!

I think that keeping a single Scrapy root logger, and using filters to separate messages based in their crawler is the way to go (that’s why I kept the ‘crawler’ key in all log messages’ meta parameter actually), but I’m hesitant of implementing those filters in Scrapy, since their usage is going to be really dependant of what the user or application wants to do with them. They can be implemented quite easily with the standard logging library outside Scrapy, and by doing that the users would hold entire control of what they want to filter and what they want to do with those filtered messages.

By any case, if we decide that we want to make that feature available in Scrapy I think we can implement it after merging this PR, since it should be something complementary that shouldn’t change the current approach.

Some notes:

In case of ScrapyRT one crawler creates exactly one spider like it expected by latest Scrapy version - crawler doesn't support having multiple Spider objects any more.

It can’t have multiple spiders running at the same time, and it’s attached to a single Spider class, but it can have different instances of that Spider class after each call of Crawler.crawl (though these calls should be sequential, can’t be done in parallel).

You say we want users to use it directly - so why Spider.logger was added?

Spider.logger was added as a convenient already created logger to avoid users the need of creating one themselves, and this attribute allows us to provide backward support for the old Spider.log method. It’s documented that this is not the only way to log things, I rewrote the logging topic in the docs to show how to use the standard logging directly.

@kmike
Copy link
Member

@kmike kmike commented Apr 10, 2015

I think passing spider or crawler as extra data for loggers is the way to go.

There are several places when crawler is passed to extra, not spider - do you think it is possible to add crawler.spider as well? Is crawler.spider initialized there?

@curita
Copy link
Member Author

@curita curita commented Apr 10, 2015

Depends, crawler.spider is not set until the end of crawler.init(), where it takes a None value, and then the actual spider instance is created while calling crawler.crawl(). There could be log messages in the extension initializations without crawler.spider being set then (if some extension is not configured is going to log a 'Disabled ' warning, for example). If 'spider' can have None as value when passing it to extra we could move crawler.spider assignment to the start of crawler.init() [crawler.py#L29-L35].

Another option to make extra parameters more uniform is to pass only 'crawler' to meta, and users could check if crawler.spider is set or not manually, or we could create a filter to populate 'spider' in extra, doing this check ourselves.

There are command-line arguments, available for all commands, that you can use
to override some of the Scrapy settings regarding logging.

* ``--logfile FILE``

This comment has been minimized.

@chekunkov

chekunkov Apr 14, 2015
Contributor

sorry, I cannot find where it's implemented

This comment has been minimized.

@chekunkov

chekunkov Apr 14, 2015
Contributor

UPD: ignore me, these are old options :)

@curita curita force-pushed the curita:python-logging branch from c1b3ce8 to 4e8ced5 Apr 14, 2015
@kmike kmike added this to the Scrapy 1.0 milestone Apr 14, 2015
curita added 20 commits Feb 28, 2015
This allows to remove `get_testlog` helper, `flushLoggedErrors` from
twisted.trial.unittest.TestCase and Twisted log observers created for
each test on conftest.py.
There are two functions, `configure_logging` and `log_scrapy_info` which
intend to replace scrapy.log.start and scrapy.log.scrapy_info
respectively.

Creating new functions makes evident the backward incompatible change of
using another logging system, and since the Python logging module is a
standard builtin, additional helpers make sense to be on a scrapy/utils
file.
@curita curita force-pushed the curita:python-logging branch from 72aa19a to 1d8f822 Apr 22, 2015
kmike added a commit that referenced this pull request Apr 29, 2015
[MRG+1] Python logging
@kmike kmike merged commit fbb1078 into scrapy:master Apr 29, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dangra
Copy link
Member

@dangra dangra commented Apr 29, 2015

LGTM

@nyov
Copy link
Contributor

@nyov nyov commented Apr 29, 2015

\o/

@pablohoffman
Copy link
Member

@pablohoffman pablohoffman commented Apr 29, 2015

🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment