You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "bschoening (via GitHub)" <gi...@apache.org> on 2023/08/31 03:18:25 UTC
[GitHub] [cassandra] bschoening opened a new pull request, #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
bschoening opened a new pull request, #2653:
URL: https://github.com/apache/cassandra/pull/2653
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bschoening commented on a diff in pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on code in PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#discussion_r1312012799
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -277,22 +276,10 @@ def __repr__(self):
return '<%s %s>' % (self.__class__.__name__, self.message())
-def maybe_ensure_text(val):
Review Comment:
Since Python 3.0, all strings are Unicode. It appears left over from python 2.x. Its used only on four command line args to "make sure user values .. are in unicode"
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "ekaterinadimitrova2 (via GitHub)" <gi...@apache.org>.
ekaterinadimitrova2 commented on PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#issuecomment-1701940526
@bschoening , @driftx - There is no test for this reported issue but field_size_limit was
introduced in - https://issues.apache.org/jira/browse/CASSANDRA-8934. It seems it was always "unused"? What do I miss? Can you take a look, please? After that one is confirmed, please feel free to commit this one. I will be off until Tuesday, no need to wait for me.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on a diff in pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "ekaterinadimitrova2 (via GitHub)" <gi...@apache.org>.
ekaterinadimitrova2 commented on code in PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#discussion_r1311841931
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -277,22 +276,10 @@ def __repr__(self):
return '<%s %s>' % (self.__class__.__name__, self.message())
-def maybe_ensure_text(val):
- return str(val) if val else val
-
-
class FormatError(DecodeError):
verb = 'format'
-def full_cql_version(ver):
- while ver.count('.') < 2:
- ver += '.0'
- ver_parts = ver.split('-', 1) + ['']
- vertuple = tuple(list(map(int, ver_parts[0].split('.'))) + [ver_parts[1]])
- return ver, vertuple
Review Comment:
The old one was handling alpha beta versions; the new one will raise a ValueError if I read it correctly.
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -277,22 +276,10 @@ def __repr__(self):
return '<%s %s>' % (self.__class__.__name__, self.message())
-def maybe_ensure_text(val):
Review Comment:
Why was this check removed?
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -2209,22 +2187,16 @@ def read_options(cmdlineargs, environment=os.environ):
else:
options.color = should_use_color()
- if options.cqlversion is not None:
- options.cqlversion, cqlvertup = full_cql_version(options.cqlversion)
- if cqlvertup[0] < 3:
- parser.error('%r is not a supported CQL version.' % options.cqlversion)
Review Comment:
Why is this removed?
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -2166,15 +2150,12 @@ def read_options(cmdlineargs, environment=os.environ):
print("\nWarning: Using a password on the command line interface can be insecure."
"\nRecommendation: use the credentials file to securely provide the password.\n", file=sys.stderr)
- # Make sure some user values read from the command line are in unicode
- options.execute = maybe_ensure_text(options.execute)
Review Comment:
Same questions as before
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bschoening commented on pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#issuecomment-1701552225
Regarding field_size_limit, it was being parsed & set but not used by the copyutil.py.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bschoening commented on pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#issuecomment-1701677221
field_size_limit seems non-functional since at least 2.1, it may have been used in 2.0 before the csv import/export was reworked.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bschoening commented on a diff in pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on code in PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#discussion_r1312038127
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -277,22 +276,10 @@ def __repr__(self):
return '<%s %s>' % (self.__class__.__name__, self.message())
-def maybe_ensure_text(val):
- return str(val) if val else val
-
-
class FormatError(DecodeError):
verb = 'format'
-def full_cql_version(ver):
- while ver.count('.') < 2:
- ver += '.0'
- ver_parts = ver.split('-', 1) + ['']
- vertuple = tuple(list(map(int, ver_parts[0].split('.'))) + [ver_parts[1]])
- return ver, vertuple
Review Comment:
The documentation at [Cassandra Query Language](https://cassandra.apache.org/doc/latest/cassandra/cql/changes.html) describes a valid version as only major/minor/patch.
> Versioning of the CQL language adheres to the [Semantic Versioning](http://semver.org/) guidelines. Versions take the form X.Y.Z where X, Y, and Z are integer values representing major, minor, and patch level respectively. There is no correlation between Cassandra release versions and the CQL language version.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bschoening commented on a diff in pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on code in PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#discussion_r1312012799
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -277,22 +276,10 @@ def __repr__(self):
return '<%s %s>' % (self.__class__.__name__, self.message())
-def maybe_ensure_text(val):
Review Comment:
Since Python 3.0, all string are Unicode. It appears left over from python 2.x. Its used only on four command line args to "make sure user values .. are in unicode"
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bschoening commented on pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#issuecomment-1703205121
As noted in the comments on the Jira, the implementation of field_size_limit is left over from version 2.0 before the copy from/copy to behavior was refactored into the copyutil.py file (2.1.13) in [CASSANDRA-9304](https://issues.apache.org/jira/browse/CASSANDRA-9304) It's been a non-functional arg in CQLSH since that time.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bschoening commented on a diff in pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on code in PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#discussion_r1312014520
##########
pylib/cqlshlib/cqlshmain.py:
##########
@@ -2209,22 +2187,16 @@ def read_options(cmdlineargs, environment=os.environ):
else:
options.color = should_use_color()
- if options.cqlversion is not None:
- options.cqlversion, cqlvertup = full_cql_version(options.cqlversion)
- if cqlvertup[0] < 3:
- parser.error('%r is not a supported CQL version.' % options.cqlversion)
Review Comment:
Passing anything other than the current CQL version (3.4.6 w/4.1) causes an error:
$ cqlsh --cqlversion 3.4.2
....
Connection error: ('Unable to connect to any servers', {'127.0.0.1:9042': ProtocolError("cql_version '3.4.2' is not supported by remote (w/ native protocol). Supported versions: ['3.4.6']")})
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ekaterinadimitrova2 commented on pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "ekaterinadimitrova2 (via GitHub)" <gi...@apache.org>.
ekaterinadimitrova2 commented on PR #2653:
URL: https://github.com/apache/cassandra/pull/2653#issuecomment-1701584082
`Regarding field_size_limit, it was being parsed if defined but not used by the copyutil.py.`
The question is, why? Is it a bug or a leftover? I think we will need to do some archeology 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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic closed pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #2653: Cassandra 18787: Replace or remove obsolete functions and simplify cql_verison handling
URL: https://github.com/apache/cassandra/pull/2653
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org