earl / beanstalkc

A simple beanstalkd client library for Python
Apache License 2.0
457 stars 115 forks source link

Change failure mode for missing YAML parser (PyYAML) dependency #49

Open earl opened 10 years ago

earl commented 10 years ago

Currently, beanstalkc for all practical purposes depends on PyYAML. Unless told otherwise, instantiating a Connection will try to import PyYAML; if that import fails, beanstalkc will log an error (using logging.error) but will continue working, by switching to a YAML-less operating mode.

I'm now convinced, that this failure mode is a mistake; it's the opposite of "fail-fast". In practice, the automatic fail-over to YAML-less operation has done more harm than good. Basically, it made (though typically brief) debugging necessary, instead of properly failing right away and making the error condition obvious.

Therefore, I plan to switch the default failure mode: in the future, when PyYAML is missing, beanstalkc won't handle the ImportError itself, but just pass it through. This is a more transparent fail-fast.

However, PyYAML won't become a hard dependency. You'll still be able to operate beanstalkc without PyYAML, explicitly switching to YAML-less operation by passing parse_yaml=False to the Connection constructor. Explicitly switching to YAML-less operation has been supported for a long time, so that's nothing new. It is already used by all real-world YAML-less deployments I know of.

So the main thing that'll change, is switching the default to a more sensible fail-fast behaviour.

Unless there is major opposition against this change, or someone suggests something better, the new default behaviour will be switched for the next feature release (beanstalkc 0.5.0).

ptjm commented 10 years ago

I don't have a strong opinion one way or another, but before you do it consider my recent pull request #51. That wasn't a response to this issue, but I guess it was my idea for dealing with the same problem.

Rogdham commented 8 years ago

:+1:

This could potentially only affect the users who do not have installed PyYAML, and still don't disable it explicitly. I would argue that they are probably not many: the logger part is set to the error level (which may not be the right level btw). This alone is pretty annoying when looking at the console during development / debug times, and quite bad during production where logs are stored, monitored, etc.

Failing shortly in that case leads to a choice being made during development, which is a good thing imho. Plus it makes beanstalkc's code cleaner, which is always nice :-)