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

Add better error message for incorrect worksheet name. #4297

Merged
merged 6 commits into from
Nov 4, 2022

Conversation

leilenah
Copy link
Collaborator

@leilenah leilenah commented Oct 30, 2022

Reasons for making this change

This change allows the correct error to surface for users who type in an incorrect password.

Currently the "incorrect password" error is being masked by the CLI.

Related issues

#4188

Screenshots

20221103233333926.mp4

Checklist

  • I've added a screenshot of the changes, if this is a frontend change
  • I've added and/or updated tests, if this is a backend change
  • I've run the pre-commit.sh script
  • I've updated docs, if needed

@leilenah leilenah linked an issue Oct 30, 2022 that may be closed by this pull request
@leilenah leilenah assigned percyliang and unassigned percyliang Oct 30, 2022
@leilenah
Copy link
Collaborator Author

@percyliang the original request was to change the message that appears if a user types in an incorrect password. However, the incorrect password error handling seems to be functioning as expected (see below).

I think that the nlp:: worksheet that you were requesting wasn't found, which was not clear in the error message that was surfaced to you as you were logging in. So I updated the error message that is shown when a worksheet is not found.

Lmk if I'm missing something.

image

@percyliang
Copy link
Collaborator

nlp:: is not the worksheet but the alias, which does exist in my case. Run cl alias nlp https://codalab.stanford.edu first.

@leilenah
Copy link
Collaborator Author

leilenah commented Nov 1, 2022

@percyliang this is what I'm seeing on my end while using my local cl (note that I typed in an incorrect password):

20221101101311125.mp4

@leilenah
Copy link
Collaborator Author

leilenah commented Nov 4, 2022

@percyliang can you re-review this please? This change will allow a PermissionError to be raised if the user has typed in an incorrect password.

Also I confirmed that if a worksheet is not found, an informative NotFoundError will still be surfaced to the user.

@leilenah leilenah merged commit 366b870 into master Nov 4, 2022
@leilenah leilenah deleted the feature/4188-cli-password-error branch November 4, 2022 07:49
@leilenah leilenah mentioned this pull request Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CLI] better error message for incorrect password
2 participants