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

ENH: message: show reason for InvalidKey exception #2398

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

wagner-intevation
Copy link
Contributor

if the user wants to add a field with an invalid key, the reason was not clear
now the error message says that either

  • the key is not allowed in the harmonization configuration or
  • the key does not match the regular expression (for extra keys), e.g. is not lower case etc.

@sebix sebix added feature Indicates new feature requests or new features usability component: core labels Aug 17, 2023
@sebix sebix added this to the 3.2.1 milestone Aug 17, 2023
Copy link
Contributor

@kamil-certat kamil-certat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved assuming you fix the last-resort return from validation function

message = "invalid key %s" % repr(key)
def __init__(self, key: str, additional_text: Optional[str] = None):
postfix = f' {additional_text}' if additional_text else ''
message = f"invalid key {key!r}{postfix}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just:

message = f"invalid key {key!r} {additional_text or ''}"

if subitem:
return HARMONIZATION_KEY_FORMAT.match(key)
return HARMONIZATION_KEY_FORMAT.match(key), f'Does not match regular expression {HARMONIZATION_KEY_FORMAT.pattern}'
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cation: Default case return just one item instead of expected tuple

@sebix sebix modified the milestones: 3.2.1, 3.2.2 Aug 28, 2023
if the user wants to add a field with an invalid key, the reason was not
clear
now the error message says that either
the key is not allowed in the harmonization configuration
or the key does not match the regular expression (for extra keys), e.g.
 is not lower case etc.
@sebix sebix merged commit 85c215d into certtools:develop Sep 6, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: core feature Indicates new feature requests or new features usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants