absurdfarce commented on code in PR #1300:
URL:
https://github.com/apache/cassandra-python-driver/pull/1300#discussion_r3617553742
##########
cassandra/cqltypes.py:
##########
@@ -1006,9 +1020,11 @@ def evict_udt_class(cls, keyspace, udt_name):
@classmethod
def apply_parameters(cls, subtypes, names):
- keyspace = subtypes[0].cass_parameterized_type() # when parsed from
cassandra type, the keyspace is created as an unrecognized cass type; This gets
the name back
+ # when parsed from cassandra type, the keyspace is created as an
unrecognized cass type; This resolves the name
+ keyspace = subtypes[0].cass_parameterized_type()
udt_name = _name_from_hex_string(subtypes[1].cassname)
- field_names = tuple(_name_from_hex_string(encoded_name) for
encoded_name in names[2:]) # using tuple here to match what comes into
make_udt_class from other sources (for caching equality test)
+ # tuple used to match what comes into make_udt_class from other
sources (for caching equality test)
Review Comment:
Nit: perhaps just "tupled matches what comes into..."; seems like that makes
things a bit more readable?
##########
cassandra/util.py:
##########
@@ -1692,15 +1709,17 @@ def __repr__(self):
self.lower_bound, self.upper_bound, self.value
)
+
VERSION_REGEX =
re.compile("^(\\d+)\\.(\\d+)(\\.\\d+)?(\\.\\d+)?([~\\-]\\w[.\\w]*(?:-\\w[.\\w]*)*)?(\\+[.\\w]+)?$")
+
@total_ordering
class Version(object):
"""
Representation of a Cassandra version. Mostly follows the implementation
of the same logic in the Java driver;
see
https://github.com/apache/cassandra-java-driver/blob/4.19.2/core/src/main/java/com/datastax/oss/driver/api/core/Version.java.
- Cassandra versions are assumed to correspond to major.minor.patch with an
optional additional numeric build field as well as a
+ Cassandra's versions are assumed to correspond to major.minor.patch with
an optional additional numeric build field as well as a
Review Comment:
I'd actually argue the text above reads more clearly but I don't feel
super-strong about it or anything
##########
cassandra/cqltypes.py:
##########
@@ -1006,9 +1020,11 @@ def evict_udt_class(cls, keyspace, udt_name):
@classmethod
def apply_parameters(cls, subtypes, names):
- keyspace = subtypes[0].cass_parameterized_type() # when parsed from
cassandra type, the keyspace is created as an unrecognized cass type; This gets
the name back
+ # when parsed from cassandra type, the keyspace is created as an
unrecognized cass type; This resolves the name
Review Comment:
"This" is still capitalized as it stands right now which seems wrong when
it's coming after the semi-colon.
##########
cassandra/metadata.py:
##########
@@ -789,16 +788,16 @@ def export_as_string(self):
other_tables = [t for t in self.tables.values() if t not in
tables_with_vertex]
cql = "\n\n".join(
- [self.as_cql_query() + ';'] +
- self.user_type_strings() +
- [f.export_as_string() for f in self.functions.values()] +
- [a.export_as_string() for a in self.aggregates.values()] +
- [t.export_as_string() for t in tables_with_vertex + other_tables])
+ [self.as_cql_query() + ';']
+ + self.user_type_strings()
+ + [f.export_as_string() for f in self.functions.values()]
+ + [a.export_as_string() for a in self.aggregates.values()]
+ + [t.export_as_string() for t in tables_with_vertex +
other_tables])
if self._exc_info:
import traceback
ret = "/*\nWarning: Keyspace %s is incomplete because of an error
processing metadata.\n" % \
- (self.name)
+ self.name
Review Comment:
This... looked odd to me 'cause I was pretty sure the % operator required a
tuple as it's arg... but apparently there's a special case for single values
(based on [the
docs](https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting)
anyway). I'm kinda wondering if maybe it isn't worth our while to move away
from the % operator anyway but I'm very receptive to an argument that says this
PR isn't the place to do that.
We can consider another ticket to move to f-strings (or perhaps some other
option) uniformly throughout the code base but it's definitely not worth
side-tracking this effort to do so here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
