Merge branch 'develop' into master

* develop:
  Update CHANGELOG.md
  [http] Re-introduce read optimization when buffer is empty (#907)
  [http] Eliminate _read_iter and simplify read (#906)

Author: ddelange

CHANGELOG.md

@@ -1,3 +1,8 @@
+# 7.4.4, 2025-11-04
+
+- [http] Eliminate _read_iter and simplify read (PR [#906](https://github.com/piskvorky/smart_open/pull/906), [@ddelange](https://github.com/ddelange))
+- [http] Re-introduce read optimization when buffer is empty (PR [#907](https://github.com/piskvorky/smart_open/pull/907), [@ddelange](https://github.com/ddelange))
+
 # 7.4.3, 2025-11-03
 
 - Simplify WHENCE_END logic (PR [#902](https://github.com/piskvorky/smart_open/pull/902), [@ddelange](https://github.com/ddelange))

smart_open/http.py

@@ -138,7 +138,6 @@ def __init__(self, url, mode='r', buffer_size=DEFAULT_BUFFER_SIZE,
         if not self.response.ok:
             self.response.raise_for_status()
 
-        self._read_iter = iter(lambda: self.response.raw.read(self.buffer_size), b"")
         self._read_buffer = bytebuffer.ByteBuffer(buffer_size)
         self._current_pos = 0
 
@@ -150,7 +149,6 @@ def close(self):
         logger.debug("close: called")
         if not self.closed:
             self.response = None
-            self._read_iter = None
             self._read_buffer = None
 
     @property
@@ -175,32 +173,23 @@ def read(self, size=-1):
         """
         Mimics the read call to a filehandle object.
         """
+        if size < -1:
+            raise ValueError(f'size must be >= -1, got {size}')
+
         logger.debug("reading with size: %d", size)
-        if self.response is None:
-            return b''
-
-        if size == 0:
-            return b''
-        elif size < 0 and len(self._read_buffer) == 0:
-            retval = self.response.raw.read()
-        elif size < 0:
-            retval = self._read_buffer.read() + self.response.raw.read()
+        if self.closed or size == 0:
+            return b""
+
+        if size == -1:
+            if len(self._read_buffer):
+                retval = self._read_buffer.read() + self.response.raw.read()
+            else:  # Avoid unnecessary +
+                retval = self.response.raw.read()
         else:
+            # Fill _read_buffer until it contains enough bytes
             while len(self._read_buffer) < size:
-                logger.debug(
-                    "http reading more content at current_pos: %d with size: %d",
-                    self._current_pos, size,
-                )
-                bytes_read = self._read_buffer.fill(self._read_iter)
-                if bytes_read == 0:
-                    # Oops, ran out of data early.
-                    retval = self._read_buffer.read()
-                    self._current_pos += len(retval)
-
-                    return retval
-
-            # If we got here, it means we have enough data in the buffer
-            # to return to the caller.
+                if self._read_buffer.fill(self.response.raw) == 0:
+                    break  # EOF reached
             retval = self._read_buffer.read(size)
 
         self._current_pos += len(retval)
@@ -281,13 +270,11 @@ def seek(self, offset, whence=0):
 
         if new_pos == self.content_length:
             self.response = None
-            self._read_iter = None
             self._read_buffer.empty()
         else:
             response = self._partial_request(new_pos)
             if response.ok:
                 self.response = response
-                self._read_iter = iter(lambda: self.response.raw.read(self.buffer_size), b"")
                 self._read_buffer.empty()
             else:
                 self.response = None

tests/test_http.py

@@ -226,30 +226,60 @@ def callback(request):
 
     responses.add_callback(responses.GET, URL, callback=callback)
     # Explicitly request gzip encoding
-    reader = smart_open.http.SeekableBufferedInputBase(
-        URL, headers={'Accept-Encoding': 'gzip'}
-    )
+    reader = smart_open.http.SeekableBufferedInputBase(URL, headers={'Accept-Encoding': 'gzip'})
     read_bytes = reader.read()
     assert read_bytes == BYTES  # Should be decompressed by requests/urllib3
 
+    # Combining multiple read calls also works
+    reader.seek(0)
+    partial = reader.read(2) + reader.read(1000)
+    assert partial == BYTES
+
 
 @responses.activate
-def test_gzip_consistency_between_read_methods():
-    """Regression test: read() and read(size) should return same decompressed data."""
-    def callback(request):
-        # Server returns gzipped data when client accepts it
-        if 'gzip' in request.headers.get('Accept-Encoding', ''):
-            headers = HEADERS.copy()
-            headers['Content-Encoding'] = 'gzip'
-            headers['Content-Length'] = str(len(GZIPPED_BYTES))
-            return (200, headers, GZIPPED_BYTES)
-        else:
-            headers = HEADERS.copy()
-            headers['Content-Length'] = str(len(BYTES))
-            return (200, headers, BYTES)
+def test_read_after_read_to_eof():
+    """Reading after reading to EOF should return empty bytes."""
+    responses.add_callback(responses.GET, URL, callback=request_callback)
+    reader = smart_open.http.SeekableBufferedInputBase(URL)
 
-    responses.add_callback(responses.GET, URL, callback=callback)
-    reader = smart_open.http.SeekableBufferedInputBase(URL, headers={'Accept-Encoding': 'gzip'})
-    partial = reader.read(2) + reader.read()
-    assert len(partial) == len(BYTES), f"Expected {len(BYTES)} bytes, got {len(partial)}"
-    assert partial == BYTES
+    # Read to EOF
+    result = reader.read(-1)
+    assert len(result) == len(BYTES)
+    assert reader.tell() == len(BYTES)
+
+    # Read should return empty bytes
+    result = reader.read()
+    assert result == b""
+
+    # Read with size should also return empty bytes
+    result = reader.read(10)
+    assert result == b""
+
+
+@responses.activate
+def test_read_after_seek_to_eof():
+    """Reading after seeking to EOF should return empty bytes."""
+    responses.add_callback(responses.GET, URL, callback=request_callback)
+    reader = smart_open.http.SeekableBufferedInputBase(URL)
+
+    # Seek to EOF
+    reader.seek(0, whence=smart_open.constants.WHENCE_END)
+    assert reader.tell() == len(BYTES)
+
+    # Read should return empty bytes
+    result = reader.read()
+    assert result == b""
+
+    # Read with size should also return empty bytes
+    result = reader.read(10)
+    assert result == b""
+
+
+@responses.activate
+def test_read_with_invalid_size():
+    """Read with size < -1 should raise ValueError."""
+    responses.add_callback(responses.GET, URL, callback=request_callback)
+    reader = smart_open.http.SeekableBufferedInputBase(URL)
+
+    with pytest.raises(ValueError, match='size must be >= -1'):
+        reader.read(-2)