Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: missing username in connection URL prevents username from appearing in prompt #10863

Closed
knz opened this issue Nov 20, 2016 · 16 comments
Closed
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-easy Easy issue to tackle, requires little or no CockroachDB experience help wanted Help is requested / needed by the one who filed the issue to fix it.
Milestone

Comments

@knz
Copy link
Contributor

knz commented Nov 20, 2016

If COCKROACH_URL or --url specifies no username with an insecure cluster, the connection is accepted and the underlying pq library uses the name of the user who launched the shell. However the CLI shell's prompt does not see this and the username part appears empty in the prompt.

A better behavior would be to query the database after the connection is established to determine what the current user is, much like the CLI already queries the database for syntax/txn status.

Found while investigating #10835.

@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Nov 20, 2016
@asubiotto
Copy link
Contributor

I don't think the username field will be empty. lib/pq uses the username provided by the operating system if none is provided through the URL. I tested it out and my os username is correctly used.

After #10862 (thanks for catching that btw) the only issue I see is that the username used for the connection won't be reflected in the prompt.

@knz
Copy link
Contributor Author

knz commented Nov 29, 2016

Oh thanks for your analysis. Then the cli shell should do what is already done for other things and fetch the current user from the database. Changed the issue description accordingly.

@knz knz changed the title cli: missing username in connection URL creates session without username cli: missing username in connection URL prevents username from appearing in prompt Nov 29, 2016
@knz knz added E-easy Easy issue to tackle, requires little or no CockroachDB experience help wanted Help is requested / needed by the one who filed the issue to fix it. labels Nov 29, 2016
@a6802739
Copy link
Contributor

@knz, I want to have a try for this.

@knz
Copy link
Contributor Author

knz commented Dec 14, 2016

@a6802739 ok, thanks!

@a6802739 a6802739 self-assigned this Dec 14, 2016
@a6802739
Copy link
Contributor

@knz, what does the prevents username from appearing in prompt prompt mean here?

@knz
Copy link
Contributor Author

knz commented Dec 15, 2016

  1. start insecure cluster
  2. run client with ./cockroach sql --url="postgresql://localhost:26257?sslmode=disable"
  3. look at prompt:
# Welcome to the cockroach SQL interface.
# All statements must be terminated by a semicolon.
# To exit: CTRL + D.
@localhost:26257>

(the username is missing)

@a6802739
Copy link
Contributor

@knz, thanks for your reference.

@pmamatsis
Copy link
Contributor

Good evening @knz may i give this is issue a go myself ?

Best regards,
Panos.

@knz
Copy link
Contributor Author

knz commented Jan 5, 2017

yes you can also have a look.

@knz
Copy link
Contributor Author

knz commented Jan 5, 2017

@pmamatsis note we have already a PR out for this in #12408. You can help with reviewing it though.

@pmamatsis
Copy link
Contributor

pmamatsis commented Feb 11, 2017

Good evening @knz,
i have managed to perform the rebase of my PR successfully. I will start working on this issue as discussed and implement it in a way to fit the existing FSM.

I wanted to ask you a newbie question about the rebase. When i am checking into master (i have added the official repository of cockroachdb as 'main') and perform the git status command i am getting the message:

On branch master
Your branch is ahead of 'origin/master' by 796 commits.
  (use "git push" to publish your local commits)
nothing to commit, working tree clean

If i execute the command git remote -v i am getting the following results:

main	https://github.com/cockroachdb/cockroach.git (fetch)
main	https://github.com/cockroachdb/cockroach.git (push)
origin	https://github.com/pmamatsis/cockroach (fetch)
origin	https://github.com/pmamatsis/cockroach (push)

Best regards,
Panos.

@knz
Copy link
Contributor Author

knz commented Feb 11, 2017 via email

@pmamatsis
Copy link
Contributor

Good afternoon @knz,
my apologies for the late reply. The contents of my .git/config are the following:

[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
[remote "origin"]
	url = https://github.com/pmamatsis/cockroach
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
	remote = origin
	merge = refs/heads/master
[submodule "vendor"]
	url = https://github.com/cockroachdb/vendored
[remote "main"]
	url = https://github.com/cockroachdb/cockroach.git
	fetch = +refs/heads/*:refs/remotes/main/*

I have checked the pkg/cli/sql.go file that i have locally against the one in the official cockroachdb repo and they are identical though.

Best regards and thank you in advance,
Panos.

@knz
Copy link
Contributor Author

knz commented Feb 13, 2017

Ok then, it's easy: the message merely indicates that your master branch on https://github.com/pmamatsis/cockroach is not up to date. By running git push you can bring it forward. This will synchronize your repository at https://github.com/pmamatsis/cockroach with ours.

@pmamatsis
Copy link
Contributor

Good evening @knz,
thank you for your prompt and quick reply. I will do this and then i will start working on the current issue.

Best regards,
Panos.

@petermattis petermattis added this to the 1.0 milestone Feb 23, 2017
@dianasaur323
Copy link
Contributor

Looks like this was fixed by #12408

Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-easy Easy issue to tackle, requires little or no CockroachDB experience help wanted Help is requested / needed by the one who filed the issue to fix it.
Projects
None yet
Development

No branches or pull requests

6 participants