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

Confusing exception message #23283

Closed
islem-esi opened this issue Mar 2, 2022 · 3 comments · Fixed by #23284
Closed

Confusing exception message #23283

islem-esi opened this issue Mar 2, 2022 · 3 comments · Fixed by #23284
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Milestone

Comments

@islem-esi
Copy link

As I was looking at the code in this File, I got confused by the exception message. I think the word "not" is mistakenly repeated in the exception message, otherwise it would be better to remove 'not not' (See code below).

   try:
        parsed_uri = parse.urlparse(source_id)
    except Exception:  # pylint: disable=broad-except
        raise ValueError("'{}' is not not a valid ID".format(source_id))
    if not (parsed_uri.scheme and parsed_uri.hostname):
        raise ValueError("'{}' is not not a valid ID".format(source_id))

Is it the case?

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Mar 2, 2022
@azure-sdk azure-sdk added Client This issue points to a problem in the data-plane of the library. KeyVault needs-team-triage Workflow: This issue needs the team to triage. labels Mar 2, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 2, 2022
@annatisch annatisch removed the needs-team-triage Workflow: This issue needs the team to triage. label Mar 2, 2022
@annatisch
Copy link
Member

Thanks so much for reporting this @islem-esi - I think you're right, this is probably a mistake, if not - double-negatives are a confusing grammar choice for an error message :)
Let's get that fixed up....

@annatisch
Copy link
Member

I opened a quick PR to fix - @mccoyp could you please do a quick review to confirm I haven't misinterpreted the error message?

@mccoyp
Copy link
Member

mccoyp commented Mar 2, 2022

Thank you @islem-esi for catching this, and thank you @annatisch for opening a PR! I'll get that approved. We definitely didn't mean to leave confusing double-negatives around 😅

@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants