search

scylladb / scylla-cqlsh

A fork of the cqlsh code
Apache License 2.0
16 stars 30 forks source link

Fix the support for separate credential file #74

Closed slavavrn closed 6 months ago

slavavrn commented 6 months ago

I have restored the ability to specify a login and password in cqlshrc. However, I discovered that the features introduced by this commit are not reflected in the documentation. If you tell me how to make changes to the documentation, I can add a description of using AuthProviders for cqlsh to the documentation. I also see shortcomings in the tests, for example cqlshlib/test/test_cqlsh_completion.py does not work correctly on an empty database. Therefore, the autotests did not pass completely. But this needs to be corrected with a separate pull request.

Fixes: #73

fruch commented 6 months ago

Regarding the test, they would work, and can generate its own data.

You'll soon see the CI run, running the suite as in the CI GitHub action, should be working also locally, if not please report in a separate issue.

fruch commented 6 months ago

Can you add titles and brief descriptions to the commits ?

A nice to have for this fix, is a test using both option of files, to validate this is fixed, and to make sure it won't break again

slavavrn commented 6 months ago

What should I do better? Close this pull request, remove the commit that changes the notice output to stdout, and create a new pull request? Or make another commit that undoes the last commit?

fruch commented 6 months ago

What should I do better? Close this pull request, remove the commit that changes the notice output to stdout, and create a new pull request? Or make another commit that undoes the last commit?

I think you can change this PR in place, and remove the change related to the output a nice to have would be a test as I mentioned, but not critical

fruch commented 6 months ago

@slavavrn did you reported this issue to cassandra as well ?

seems like they are suffering from the same exact issue

slavavrn commented 6 months ago

Maybe I did it wrong, but I replaced the commit so that there were no erroneous commits

slavavrn commented 6 months ago

@slavavrn did you reported this issue to cassandra as well ?

seems like they are suffering from the same exact issue

No, I didn't do that. Sorry, I don’t understand very well, do I need to duplicate the issue in Cassandra, or can I make a pull request from here?

fruch commented 6 months ago

@slavavrn did you reported this issue to cassandra as well ?

seems like they are suffering from the same exact issue

No, I didn't do that. Sorry, I don’t understand very well, do I need to duplicate the issue in Cassandra, or can I make a pull request from here?

I'm guessing the fix is similar, and you can open the PR on the Cassandra repository

They usually asks for an issue on their system

slavavrn commented 6 months ago

I added a test to check credentials

slavavrn commented 6 months ago

I also requested registration on the cassandra issues tracker in order to create an issue there