You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "frankgh (via GitHub)" <gi...@apache.org> on 2024/12/31 17:58:22 UTC
[PR] Fix autocompletion for role names/user names [cassandra]
frankgh opened a new pull request, #3766:
URL: https://github.com/apache/cassandra/pull/3766
patch by Francisco Guerrero; reviewed by TBD for CASSANDRA-20175
--
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
Re: [PR] Fix autocompletion for role names/user names [cassandra]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #3766:
URL: https://github.com/apache/cassandra/pull/3766#discussion_r1901200458
##########
pylib/cqlshlib/cql3handling.py:
##########
@@ -1611,32 +1611,23 @@ def permission_completer(ctxt, _):
@completer_for('username', 'name')
def username_name_completer(ctxt, cass):
- def maybe_quote(name):
- if CqlRuleSet.is_valid_cql3_name(name):
- return name
- return "'%s'" % name
-
# disable completion for CREATE USER.
if ctxt.matched[0][1].upper() == 'CREATE':
return [Hint('<username>')]
session = cass.session
- return [maybe_quote(list(row.values())[0].replace("'", "''")) for row in session.execute("LIST USERS")]
+ return map(maybe_escape_name, [row['name'] for row in session.execute("LIST USERS")])
Review Comment:
`protect_name` internally calls `maybe_escape_name`. Maybe I'm missing something, but what would be the benefit of using `protect_name`?
--
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
Re: [PR] Fix autocompletion for role names/user names [cassandra]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #3766:
URL: https://github.com/apache/cassandra/pull/3766#discussion_r1901205788
##########
pylib/cqlshlib/cql3handling.py:
##########
@@ -1611,32 +1611,23 @@ def permission_completer(ctxt, _):
@completer_for('username', 'name')
def username_name_completer(ctxt, cass):
- def maybe_quote(name):
- if CqlRuleSet.is_valid_cql3_name(name):
- return name
- return "'%s'" % name
-
# disable completion for CREATE USER.
if ctxt.matched[0][1].upper() == 'CREATE':
return [Hint('<username>')]
session = cass.session
- return [maybe_quote(list(row.values())[0].replace("'", "''")) for row in session.execute("LIST USERS")]
+ return map(maybe_escape_name, [row['name'] for row in session.execute("LIST USERS")])
Review Comment:
to me it feels like it's more of a legacy method name : https://github.com/datastax/python-driver/commit/0959b5dc1662d6957f2951738d0a4f053ac78f66 and not really a higher abstraction. But I don't have a lot of context on the history of these method names
--
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
Re: [PR] Fix autocompletion for role names/user names [cassandra]
Posted by "bbotella (via GitHub)" <gi...@apache.org>.
bbotella commented on PR #3766:
URL: https://github.com/apache/cassandra/pull/3766#issuecomment-2568286172
+1 (nb)
--
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
Re: [PR] Fix autocompletion for role names/user names [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #3766: Fix autocompletion for role names/user names
URL: https://github.com/apache/cassandra/pull/3766
--
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
Re: [PR] Fix autocompletion for role names/user names [cassandra]
Posted by "bbotella (via GitHub)" <gi...@apache.org>.
bbotella commented on code in PR #3766:
URL: https://github.com/apache/cassandra/pull/3766#discussion_r1901202480
##########
pylib/cqlshlib/cql3handling.py:
##########
@@ -1611,32 +1611,23 @@ def permission_completer(ctxt, _):
@completer_for('username', 'name')
def username_name_completer(ctxt, cass):
- def maybe_quote(name):
- if CqlRuleSet.is_valid_cql3_name(name):
- return name
- return "'%s'" % name
-
# disable completion for CREATE USER.
if ctxt.matched[0][1].upper() == 'CREATE':
return [Hint('<username>')]
session = cass.session
- return [maybe_quote(list(row.values())[0].replace("'", "''")) for row in session.execute("LIST USERS")]
+ return map(maybe_escape_name, [row['name'] for row in session.execute("LIST USERS")])
Review Comment:
Basically, just use the "higher abstraction" provided in the driver? It is just a nit, but I am curious to why `protect_name` even exists.
--
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
Re: [PR] Fix autocompletion for role names/user names [cassandra]
Posted by "bbotella (via GitHub)" <gi...@apache.org>.
bbotella commented on code in PR #3766:
URL: https://github.com/apache/cassandra/pull/3766#discussion_r1901185345
##########
pylib/cqlshlib/cql3handling.py:
##########
@@ -1611,32 +1611,23 @@ def permission_completer(ctxt, _):
@completer_for('username', 'name')
def username_name_completer(ctxt, cass):
- def maybe_quote(name):
- if CqlRuleSet.is_valid_cql3_name(name):
- return name
- return "'%s'" % name
-
# disable completion for CREATE USER.
if ctxt.matched[0][1].upper() == 'CREATE':
return [Hint('<username>')]
session = cass.session
- return [maybe_quote(list(row.values())[0].replace("'", "''")) for row in session.execute("LIST USERS")]
+ return map(maybe_escape_name, [row['name'] for row in session.execute("LIST USERS")])
Review Comment:
What about using the wrapper `protect_name`?
https://github.com/datastax/python-driver/blob/master/cassandra/metadata.py#L1567
--
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