adress review comments

hazelcast/protocol/builtin.py

@@ -577,7 +577,7 @@ def decode(msg, item_size, decoder):
             header_size = ListCNFixedSizeCodec._HEADER_SIZE
             return [decoder(frame.buf, header_size + i * item_size) for i in range(count)]
         else:
-            response = []
+            response = [None] * count
             position = ListCNFixedSizeCodec._HEADER_SIZE
             read_count = 0
             items_per_bitmask = ListCNFixedSizeCodec._ITEMS_PER_BITMASK
@@ -590,12 +590,10 @@ def decode(msg, item_size, decoder):
                 for i in range(batch_size):
                     mask = 1 << i
                     if (bitmask & mask) == mask:
-                        response.append(decoder(frame.buf, position))
+                        response[read_count] = decoder(frame.buf, position)
                         position += item_size
-                    else:
-                        response.append(None)
 
-                read_count += batch_size
+                    read_count += 1
 
             return response
 
@@ -706,48 +704,47 @@ def decode(msg):
 
         # read column types
         column_type_ids = ListIntegerCodec.decode(msg)
+        column_count = len(column_type_ids)
 
         # read columns
-        columns = []
+        columns = [None] * column_count
+
+        for i in range(column_count):
+            column_type_id = column_type_ids[i]
 
-        for column_type_id in column_type_ids:
             if column_type_id == SqlColumnType.VARCHAR:
-                columns.append(
-                    ListMultiFrameCodec.decode_contains_nullable(msg, StringCodec.decode)
-                )
+                columns[i] = ListMultiFrameCodec.decode_contains_nullable(msg, StringCodec.decode)
             elif column_type_id == SqlColumnType.BOOLEAN:
-                columns.append(ListCNBooleanCodec.decode(msg))
+                columns[i] = ListCNBooleanCodec.decode(msg)
             elif column_type_id == SqlColumnType.TINYINT:
-                columns.append(ListCNByteCodec.decode(msg))
+                columns[i] = ListCNByteCodec.decode(msg)
             elif column_type_id == SqlColumnType.SMALLINT:
-                columns.append(ListCNShortCodec.decode(msg))
+                columns[i] = ListCNShortCodec.decode(msg)
             elif column_type_id == SqlColumnType.INTEGER:
-                columns.append(ListCNIntegerCodec.decode(msg))
+                columns[i] = ListCNIntegerCodec.decode(msg)
             elif column_type_id == SqlColumnType.BIGINT:
-                columns.append(ListCNLongCodec.decode(msg))
+                columns[i] = ListCNLongCodec.decode(msg)
             elif column_type_id == SqlColumnType.REAL:
-                columns.append(ListCNFloatCodec.decode(msg))
+                columns[i] = ListCNFloatCodec.decode(msg)
             elif column_type_id == SqlColumnType.DOUBLE:
-                columns.append(ListCNDoubleCodec.decode(msg))
+                columns[i] = ListCNDoubleCodec.decode(msg)
             elif column_type_id == SqlColumnType.DATE:
-                columns.append(ListCNLocalDateCodec.decode(msg))
+                columns[i] = ListCNLocalDateCodec.decode(msg)
             elif column_type_id == SqlColumnType.TIME:
-                columns.append(ListCNLocalTimeCodec.decode(msg))
+                columns[i] = ListCNLocalTimeCodec.decode(msg)
             elif column_type_id == SqlColumnType.TIMESTAMP:
-                columns.append(ListCNLocalDateTimeCodec.decode(msg))
+                columns[i] = ListCNLocalDateTimeCodec.decode(msg)
             elif column_type_id == SqlColumnType.TIMESTAMP_WITH_TIME_ZONE:
-                columns.append(ListCNOffsetDateTimeCodec.decode(msg))
+                columns[i] = ListCNOffsetDateTimeCodec.decode(msg)
             elif column_type_id == SqlColumnType.DECIMAL:
-                columns.append(
-                    ListMultiFrameCodec.decode_contains_nullable(msg, BigDecimalCodec.decode)
-                )
+                columns[i] = ListMultiFrameCodec.decode_contains_nullable(msg, BigDecimalCodec.decode)
             elif column_type_id == SqlColumnType.NULL:
                 frame = msg.next_frame()
                 size = FixSizedTypesCodec.decode_int(frame.buf, 0)
                 column = [None for _ in range(size)]
-                columns.append(column)
+                columns[i] = column
             elif column_type_id == SqlColumnType.OBJECT:
-                columns.append(ListMultiFrameCodec.decode_contains_nullable(msg, DataCodec.decode))
+                columns[i] = ListMultiFrameCodec.decode_contains_nullable(msg, DataCodec.decode)
             else:
                 raise ValueError("Unknown type %s" % column_type_id)
 

hazelcast/sql.py

@@ -20,7 +20,7 @@
 class SqlService(object):
     """A service to execute SQL statements.
 
-    The service allows you to query data stored in
+    The service allows you to query data stored in a
     :class:`Map <hazelcast.proxy.map.Map>`.
 
     Warnings:
@@ -50,13 +50,13 @@ class SqlService(object):
     - For non-Portable objects, public getters and fields are used to
       populate the column list. For getters, the first letter is converted
       to lower case. A getter takes precedence over a field in case of naming
-      conflict
+      conflict.
     - For :class:`Portable <hazelcast.serialization.api.Portable>` objects,
       field names used in the
       :func:`write_portable() <hazelcast.serialization.api.Portable.write_portable>`
-      method are used to populate the column list
+      method are used to populate the column list.
 
-    The whole key and value objects could be accessed through a special fields
+    The whole key and value objects could be accessed through special fields
     ``__key`` and ``this``, respectively. If key (value) object has fields,
     then the whole key (value) field is exposed as a normal field. Otherwise the
     field is hidden. Hidden fields can be accessed directly, but are not returned
@@ -86,11 +86,11 @@ def write_portable(self, writer):
 
     This model will be resolved to the following table columns:
 
-    - ``person_id`` ``BIGINT``
-    - ``department_id`` ``BIGINT``
-    - ``name`` ``VARCHAR``
-    - ``__key`` ``OBJECT`` (hidden)
-    - ``this`` ``OBJECT`` (hidden)
+    - person_id ``BIGINT``
+    - department_id ``BIGINT``
+    - name ``VARCHAR``
+    - __key ``OBJECT`` (hidden)
+    - this ``OBJECT`` (hidden)
 
     **Consistency**
 
@@ -120,7 +120,7 @@ def write_portable(self, writer):
 
 
     See the documentation of the :class:`SqlResult` for more information about
-    the different type of iteration methods.
+    different iteration methods.
 
     Notes:
 
@@ -214,7 +214,7 @@ def from_uuid(cls, member_uuid):
 
 
 class SqlColumnMetadata(object):
-    """Metadata for one of the columns of the returned rows."""
+    """Metadata of a column in an SQL row."""
 
     __slots__ = ("_name", "_type", "_nullable")
 
@@ -235,8 +235,8 @@ def type(self):
 
     @property
     def nullable(self):
-        """bool: ``True`` if the rows in this column might be ``None``,
-        ``False`` otherwise.
+        """bool: ``True`` if this column values can be ``None``, ``False``
+        otherwise.
         """
         return self._nullable
 
@@ -287,7 +287,7 @@ def is_last(self):
         """bool: Whether this is the last page or not."""
         return self._is_last
 
-    def get_column_value(self, column_index, row_index):
+    def get_value(self, column_index, row_index):
         """
         Args:
             column_index (int):
@@ -468,7 +468,7 @@ def columns(self):
 
     @property
     def column_count(self):
-        """int: Number of column in the row."""
+        """int: Number of columns in the row."""
         return len(self._columns)
 
     def get_column(self, index):
@@ -502,7 +502,7 @@ def __repr__(self):
 
 
 class SqlRow(object):
-    """One of the rows of the SQL query result."""
+    """One of the rows of an SQL query result."""
 
     __slots__ = ("_row_metadata", "_row")
 
@@ -511,13 +511,13 @@ def __init__(self, row_metadata, row):
         self._row = row
 
     def get_object(self, column_name):
-        """Gets the value of the column by column name.
+        """Gets the value in the column indicated by the column name.
 
         Column name should be one of those defined in :class:`SqlRowMetadata`,
         case-sensitive. You may also use :func:`SqlRowMetadata.find_column` to
         test for column existence.
 
-        The class of the returned value depends on the SQL type of the column.
+        The type of the returned value depends on the SQL type of the column.
         No implicit conversions are performed on the value.
 
         Args:
@@ -596,7 +596,7 @@ def __init__(self, row_metadata, row_page, update_count):
         """
 
         self.update_count = update_count
-        """int: Update count or -1 if row metadata or row page exist."""
+        """int: Update count or -1 if the result is a rowset."""
 
 
 class _IteratorBase(object):
@@ -658,7 +658,7 @@ def _get_current_row(self):
         # The column might contain user objects so we have to deserialize it.
         # Deserialization is no-op if the value is not Data.
         return [
-            self.deserialize_fn(self.page.get_column_value(i, self.position))
+            self.deserialize_fn(self.page.get_value(i, self.position))
             for i in range(self.page.column_count)
         ]
 
@@ -695,7 +695,7 @@ def _has_next_continuation(self, future):
         if not has_next:
             # Iterator is exhausted, raise this to inform the user.
             # If the user continues to call next, we will continuously
-            # will raise this.
+            # raise this.
             raise StopIteration
 
         row = self._get_current_row()
@@ -826,12 +826,12 @@ def on_next_row(row_future):
 
     When in doubt, use the blocking API shown in the first code sample.
 
-    Also, one might call :func:`close` over the result object to
+    One can call :func:`close` method of a result object to
     release the resources associated with the result on the server side.
     It might also be used to cancel query execution on the server side
     if it is still active.
 
-    When the blocking API is used, one might also use it with ``with``
+    When the blocking API is used, one might also use ``with``
     statement to automatically close the query even if an exception
     is thrown in the iteration. ::
 
@@ -841,11 +841,13 @@ def on_next_row(row_future):
                 print(row)
 
 
-    To get the update count, use the :func:`update_count`. ::
+    To get the number of rows updated by the query, use the
+    :func:`update_count`. ::
 
         update_count = client.sql.execute("SELECT ...").update_count().result()
 
-    One does not have to call :func:`close` in this case.
+    One does not have to call :func:`close` in this case, because the result
+    will already be closed in the server-side.
     """
 
     def __init__(self, sql_service, connection, query_id, cursor_buffer_size, execute_future):
@@ -857,7 +859,7 @@ def __init__(self, sql_service, connection, query_id, cursor_buffer_size, execut
         that the execute request is made to."""
 
         self._query_id = query_id
-        """_SqlQueryId: Uniuqe id of the SQL query."""
+        """_SqlQueryId: Unique id of the SQL query."""
 
         self._cursor_buffer_size = cursor_buffer_size
         """int: Size of the cursor buffer measured in the number of rows."""
@@ -1154,7 +1156,8 @@ def _handle_execute_response(self, future):
             # the server, invocation failed.
             self._on_execute_error(self._sql_service.re_raise(e, self._connection))
 
-    def _handle_response_error(self, error):
+    @staticmethod
+    def _handle_response_error(error):
         """If the error is not ``None``, return it as
         :class:`HazelcastSqlError` so that we can raise
         it to user.
@@ -1185,7 +1188,7 @@ def _on_execute_error(self, error):
             self._execute_response.set_exception(error)
 
     def _on_execute_response(self, row_metadata, row_page, update_count):
-        """Called when the first execute request is succeed.
+        """Called when the first execute request is succeeded.
 
         Args:
             row_metadata (SqlRowMetadata): The row metadata. Might be ``None``
@@ -1344,14 +1347,14 @@ def re_raise(self, error, connection):
         so that it can be raised to the user.
 
         Args:
-            error (Exception): The error to re raise.
+            error (Exception): The error to reraise.
             connection (hazelcast.connection.Connection): Connection
                 that the query requests are routed to. If it is not
                 live, we will inform the user about the possible
                 cluster topology change.
 
         Returns:
-            HazelcastSqlError: The re raised error.
+            HazelcastSqlError: The reraised error.
         """
         if not connection.live:
             return HazelcastSqlError(

hazelcast/util.py

@@ -325,11 +325,11 @@ def can_get_next_data_member(self):
 
     def _listener(self, _):
         members = self._cluster_service.get_members()
-        data_members = []
-
-        for member in members:
-            if not member.lite_member:
-                data_members.append(member)
+        data_members = [
+            member
+            for member in members
+            if not member.lite_member
+        ]
 
         self._members = _Members(members, data_members)
 
@@ -415,7 +415,7 @@ def int_from_bytes(buffer):
         if buffer[0] & 0x80:
             neg = bytearray()
             for c in buffer:
-                neg.append(c ^ 0xFF)
+                neg.append(~c)
             return -1 * int(binascii.hexlify(neg), 16) - 1
         return int(binascii.hexlify(buffer), 16)