delay s3 request until read

[Gapex]

delay request seek until read

Motivation

• Fixes s3 open seek operation try read rest of file into buffer, which makes following read has timeout risk #362

[vuryleo]

i think adding some test would be better

[mpenkov]

Looks like a very solid start.

I left you some comments, please have a look.

[mpenkov]

Minor English fixup.

Suggested change

	# the underlying connection of streaming body was closed by peer
	# The underlying connection of the streaming body was closed by the remote peer.

[mpenkov]

Suggested change

	# close the current one explicitly, prepare to open a new and try to read again
	# Close the current connection explicitly, prepare to open a new one and try to read again.

[mpenkov]

You're setting up an infinite recursion here, in theory.

Can you think of a way to break the cycle? We could count the number of consecutive IncompleteReadErrors, and then bail out if we see more than e.g. 10.

[Gapex]

What about adding a member field to count the number of retry, when counter less than the max limit, retry, other wise, terminate the action by raising an error?

[mpenkov]

That's OK, but you need to make sure you're counting consecutive failures, as opposed to total failures. So, you reset the counter to zero on each successful attempt.

[mpenkov]

Looks like your tests are failing because you're dereferencing None, please have a look at the buildbot output (https://travis-ci.org/RaRe-Technologies/smart_open/jobs/610158039?utm_medium=notification&utm_source=github_status).

[vuryleo]

how about closed?

[Gapex]

After init, the self._body is None, the connection is not built until the first read.

[vuryleo]

So you can still read from a closed reading stream, interesting. Does this work as designed?

[Gapex]

Yes, we could read from a closed stream, even if after the close() operation, but there is no such api offer to users.
In addition, according to the botocore reference, streaming body doesn't have the closed attribute.

[vuryleo]

do we need this?

[Gapex]

I will delete the line.

[Gapex]

@mpenkov , When executing python setup.py test, read from a closed streaming body always returns 0 bytes instead of raising an IncompleteReadError, thus the action of reopening an new streaming body had not been triggered. Thus, the SeekableBufferedInputBase thinks the raw reader reaches it's EOF, and just returns the rest data in the buffer.

This makes the unit test be failed.

[mpenkov]

OK, some questions:

• Who returns 0 bytes when reading from a closed stream?
• What depends on the existing behavior (return 0 bytes)?
• The correct behavior is to raise an exception, right?

We probably want to fix this, as it deviates from the behavior of file streams in the stdlib.

[mpenkov]

@Gapex What happened here? Why did you close the PR?

[Gapex]

Yes, I will open a new pull request.