A lot of miscellaneous work #14

Open
wants to merge 35 commits into
from

Conversation

Projects
None yet
8 participants
@PeterScott

Nice project you've got! I made a bunch of changes here:

  • Merge in the wip-reconnect branch, which gives a big improvement in robustness.
  • Rename README and TUTORIAL to .md files, for GitHub user-friendliness, and expand the README.
  • Add a connection timeout. We can't do a general socket timeout, but we can add a timeout to the socket's initial connection attempt, which could be useful.
  • Get rid of the dependency on PyYaml, replacing it with a custom parser that's more compatible with beanstalkd, and about 30 times faster. Adjust documentation to match.
  • Add basic docstrings to all public methods.

There was also a brief attempt to make connections thread-local, but that patch was reverted. The Right Thing in this case is probably to keep thread-local connection pools, like they do over in redis-py.

Anyway, you've made what is easily the best beanstalkd client for Python, so thanks for that.

earl and others added some commits May 22, 2010

Move Job body reading to a central location
This further centralises socket-related errors to originate either from
within Connection#_interact or Connection#connect.
Add closed property and adapt connect
`Connection#connect` is now a no-op for a non-closed connection.

The closed property is not yet correctly maintained in the case of
socket errors.
Peter Scott
Make connections thread-local.
By making Connection inherit from threading.local, we can make a
single connection object and use it from several threads, safely.
This is enormously useful when using libraries like eventlet, and
is common in similar libraries.
Peter Scott
Add connection timeout.
We can't use a general socket timeout because of our use of buffered
socket I/O with socket.makefile(), but we can add a timeout to the
part where we connect to the server, and remove the timeout once the
connection succeeds.
Peter Scott
Switched from PyYaml to custom parser.
We can get rid of the bothersome PyYaml dependency by using a very
simple regexp-based parser, which happens to be a lot faster than
a full YAML parser. It also allows us to work around some oddities
in the way beanstalkd emits YAML: if a tube name looks like a
number, the new parser will still parse it as a string, which is
probably what users expect to happen.
Peter Scott
Add basic docstrings to all public methods.
The docstrings don't tell everything, and so there's a need for
some more extensive documentation, but they serve as a basic guide
to what each function does, which is very helpful.
Peter Scott
Switched from 70 to 80 column fill, for consistency.
Docstrings and README.md were wrapped at 70 columns, which was
not consistent with TUTORIAL.md, and uncomfortably short.
Peter Scott
Expand tutorial slightly.
Add a title at the top, and include a note about what exactly you
can stick in a job body -- arbitrary bytes -- and how to make this
play nice with Unicode.
Peter Scott
Add `version` to the parser's list of known non-numeric keys.
The parser has a list of keys which it will not attempt to parse
as numbers, in order to correctly parse messages from beanstalkd.
The version, which may look like a number in the future, should
be part of this list.
Peter Scott
Add constant-time and exponential backoff options.
When using a connection with exponential or constant backoff, you
should pass connect_automatically=False to the constructor, and
call connect() manually. This may fail. If you then call connect()
again, there will be a small delay before it attempts to reconnect.
If it fails again, the next delay will be larger, and so on,
exponentially. A successful connection resets this.
Peter Scott
Added None as the default reconnect strategy.
This behaves the same as the behavior in the main branch. Also, to
simplify the API, it connects automatically iff the reconnect strategy
is exp_backoff or constant.
@earl

This comment has been minimized.

Show comment Hide comment
@earl

earl Jan 28, 2011

Owner

Peter, thanks a lot for this contribution. I'll review and handle it in separate parts over the next few days.

First up, the easiest part: the very documentation improvements are merged in 07a9b29.

Owner

earl commented Jan 28, 2011

Peter, thanks a lot for this contribution. I'll review and handle it in separate parts over the next few days.

First up, the easiest part: the very documentation improvements are merged in 07a9b29.

@earl

This comment has been minimized.

Show comment Hide comment
@earl

earl Jan 28, 2011

Owner

I merged the connection timeout parameter in cc22cef. This is a good idea, even independently of the reconnection stuff in wip-reconnect.

Note that I made two minor modifications:

  • I renamed the parameter to connect_timeout
  • Instead of a custom default timeout, we're now falling back on socket.getdefaulttimeout (which is typically None, unless set by the user)
Owner

earl commented Jan 28, 2011

I merged the connection timeout parameter in cc22cef. This is a good idea, even independently of the reconnection stuff in wip-reconnect.

Note that I made two minor modifications:

  • I renamed the parameter to connect_timeout
  • Instead of a custom default timeout, we're now falling back on socket.getdefaulttimeout (which is typically None, unless set by the user)
@arturhoo

This comment has been minimized.

Show comment Hide comment
@arturhoo

arturhoo Aug 10, 2011

Any updates on this?

Any updates on this?

@dsully

This comment has been minimized.

Show comment Hide comment
@dsully

dsully Feb 24, 2012

I would also love to see these fixes merged.

Thanks.

dsully commented Feb 24, 2012

I would also love to see these fixes merged.

Thanks.

@dlo

This comment has been minimized.

Show comment Hide comment
@dlo

dlo Mar 22, 2012

Any update here?

dlo commented Mar 22, 2012

Any update here?

@earl

This comment has been minimized.

Show comment Hide comment
@earl

earl Mar 22, 2012

Owner

Dan and Dan, thanks for your comments. Is there anything specific you are particularly interested in seeing merged?

Owner

earl commented Mar 22, 2012

Dan and Dan, thanks for your comments. Is there anything specific you are particularly interested in seeing merged?

@PeterScott

This comment has been minimized.

Show comment Hide comment
@PeterScott

PeterScott Mar 23, 2012

The thread-safety stuff is the part that I personally care about the most.

The thread-safety stuff is the part that I personally care about the most.

@dsully

This comment has been minimized.

Show comment Hide comment
@dsully

dsully Mar 23, 2012

Thread-safety & reconnects for me.

dsully commented Mar 23, 2012

Thread-safety & reconnects for me.

@dsully

This comment has been minimized.

Show comment Hide comment
@dsully

dsully Dec 13, 2012

Ping.

dsully commented Dec 13, 2012

Ping.

@hurrycane

This comment has been minimized.

Show comment Hide comment
@hurrycane

hurrycane Mar 16, 2013

Hey Guys! I would love thread safety too!

Hey Guys! I would love thread safety too!

@hit9

This comment has been minimized.

Show comment Hide comment
@hit9

hit9 Jul 29, 2014

Ping

hit9 commented Jul 29, 2014

Ping

@ultrabug

This comment has been minimized.

Show comment Hide comment
@ultrabug

ultrabug Aug 12, 2014

+1 also for reconnection / thread safety

@earl : is it because the PR is too big that you hesitate in going forward towards this ? how could we help you with it mate ?

Thanks

+1 also for reconnection / thread safety

@earl : is it because the PR is too big that you hesitate in going forward towards this ? how could we help you with it mate ?

Thanks

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