-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-5115 Type validation errors should include the invalid type name #2085
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
Changes from 1 commit
d763664
85e860a
712bf92
0aa91d2
9127774
c071b08
117e4ac
c2cd6c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,7 +161,7 @@ def _password_digest(username: str, password: str) -> str: | |
if len(password) == 0: | ||
raise ValueError("password can't be empty") | ||
if not isinstance(username, str): | ||
raise TypeError("username must be an instance of str") | ||
raise TypeError(f"username must be an instance of str, not {type(username)}.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is customary to omit the period in these error messages. Do you mind changing these from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually is seems there are examples of both in our codebase so either way is fine with me though I still have a slight preference for omitting it. @sleepyStick I'll defer to your opinion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I like omitting it as well, lemme clean it up and then I'll merge it :) |
||
|
||
md5hash = hashlib.md5() # noqa: S324 | ||
data = f"{username}:mongo:{password}" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the changes in this file (and some other asynchronous files) aren't reflected in sync, did you run
prepre-commit run --all-files
before committing?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I did but not getting any changes in sync files.
$ pre-commit run --all-files
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check toml...............................................................Passed
check yaml...............................................................Passed
debug statements (python)................................................Passed
fix end of files.........................................................Passed
forbid new submodules................................(no files to check)Skipped
trim trailing whitespace.................................................Passed
synchro..................................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed
blacken-docs.............................................................Passed
rst
code
is two backticks............................................Passedrst directives end with two colons.......................................Passed
rst
inline code
next to normal text..................................Passedrstcheck.................................................................Passed
Validate GitHub Workflows................................................Passed
Validate GitHub Actions..............................(no files to check)Skipped
Validate Dependabot Config (v2)..........................................Passed
codespell................................................................Passed
executable-shell.........................................................Passed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have invited you to access my forked repo, clone it, check out the
detailed-error-msg
branch, run pre-commit in your environment and if it works then push the changes here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sleepyStick!
Please help to merge this PR.
Does the
tools\synchro.py
migrate error message changes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry just seeing this, let me try this now :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just ran pre-commit on my local copy of your branch and synchro looked like this (which is what i expected)
just pushed those changes :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for listening, you are a gem!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, then something is wrong with my setup.