CASSANDRA-20021 Add cqlsh autocompletion for the identity mapping feature #4408
Conversation
pylib/cqlshlib/cql3handling.py
Outdated
|
|
||
|
|
||
| syntax_rules += r''' | ||
| <addIdentityStatement> ::= "ADD" "USER" ("IF" "NOT" "EXISTS")? <stringLiteral> |
There was a problem hiding this comment.
the statement is ADD IDENTITY IF NOT EXISTS '<identity>' TO ROLE '<role>' similarly for drop statement.
There was a problem hiding this comment.
ACK. Changes also reflected in test file
Let me know if a grammar should be expressed specifically for identity instead of using stringliteral
|
|
||
| def test_expect_str_literal_autocomplete_add(self): | ||
| self.trycompletions('ADD IDENTITY ', | ||
| choices=['<pgStringLiteral>', '<quotedStringLiteral>', 'IF']) |
There was a problem hiding this comment.
self.trycompletions('ADD IDENTITY IF NOT EXISTS',
choices=['<pgStringLiteral>', '<quotedStringLiteral>'])
?
There was a problem hiding this comment.
pgStringLiteral and quotedStringLiteral are choices that autocomplete upon TAB after IDENTITY. This is for the user to check the type for their string when defining an IDENTITY.
This test can now be found in test_cqlsh_completion.py test_complete_in_add_identity, on line 1224.
| choices=[';']) | ||
|
|
||
| def test_autocomplete_drop(self): | ||
| self.trycompletions('DROP ', |
There was a problem hiding this comment.
Hah! This is the same we are testing here
https://github.com/apache/cassandra/blob/trunk/pylib/cqlshlib/test/test_cqlsh_completion.py#L615-L618
I wonder then why that one is not failing. I think we should consolidate those.
There was a problem hiding this comment.
test_complete_in_drop now expects IDENTITY and passes with it https://github.com/apache/cassandra/blob/09303597034af528c544bc9c1fb083c99cb33529/pylib/cqlshlib/test/test_cqlsh_completion.py#L613C5-L613C37
| other_choices_ok=True) | ||
|
|
||
| def test_complete_user_full_statement_add(self): | ||
| self.trycompletions("ADD IDENTITY IF NOT EXISTS '[email protected]' TO ROLE data_engineer ", |
There was a problem hiding this comment.
Test the same without "IF NOT EXISTS" maybe?
There was a problem hiding this comment.
Good idea - test passing with addition. Can find here:
| from .test_cqlsh_completion import CqlshCompletionCase | ||
|
|
||
|
|
||
| class TestIdentityMappingsCompletion(CqlshCompletionCase): |
There was a problem hiding this comment.
While I like the fact that we are splitting it into a new test file, I see that the pattern is to have them all consolidated in test_cqlsh_completion.py (that would have helped catching the DROP completion test duplication).
I think we should add these tests there to follow the existing pattern, and maybe start a conversation on the dev slack (or Mailing list?) about splitting the completion tests (right now, a python file of 1242 lines and counting).
There was a problem hiding this comment.
I agree here with Bernardo. We should probably try to consolidate the testing logic in test_cqlsh_completion.py. I think some existing autocompletion test scenarios will fail in test_cqlsh_completion.py.
I don't think we need a mailing list discussion for this change, I think it's pretty straightforward to make the change in test_cqlsh_completion.py, and we shouldn't really think too much about it.
There was a problem hiding this comment.
ACK - identity mapping test file deleted in latest commit. Tests consolidated into test_cqlsh_completion.py. New tests are TestCqlshCompletion:test_complete_in_add_identity and TestCqlshCompletion: test_complete_in_drop_identity. Fixes made to other existing tests in this file to account for IDENTITY addition.
|
And thanks for the contribution @shalnisundram! These UX improvements are a huge benefit that enhance usability and discoverability of everything Cassandra has to offer. |
shalnisundram
force-pushed
the
CASSANDRA-20021
branch
from
96680b6 to
ee5eeb5
Compare
|
+1 pending on CI. Thanks! |
…cqlsh_completion.py
|
Closed via accdaae |
4 participants
Add support for keyword autocompletion following ADD and DROP in cqlsh.
This change adds the IdentityStatement grammars needed in cql3handling.py in order to support keyword suggestions upon for identity assignments and drops. Accompanying test suite included.
See original CASSANDRA-20021 JIRA ticket for commit history and example functionality.
patch by Shalni Sundram