-
Notifications
You must be signed in to change notification settings - Fork 11
introduce helper to normalize workspace/project roles from strings #284
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
base: master
Are you sure you want to change the base?
Conversation
harminius
left a comment
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.
Ellegant 👍
varmar05
left a comment
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.
Looking good, just some minor suggestions.
|
|
||
|
|
||
| def test_string_roles(): | ||
| assert normalize_role("guest", WorkspaceRole) == WorkspaceRole.GUEST |
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.
maybe use pytest parametrize
| self.check_collaborators_members_support() | ||
| role_enum = normalize_role(workspace_role, WorkspaceRole) | ||
| if role_enum is None: | ||
| raise ValueError("bad role") |
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.
Let's create more user friendly message, e.g. f'Invalid role: {workspace_role}'
| mp.log.error(f"Error during tmp dir cleanup: {tmp_dir.name}: {e}") | ||
|
|
||
|
|
||
| def normalize_role(role: Union[str, Enum], enum_cls: Type[Enum]) -> Optional[Enum]: |
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.
add some docstring
| """ | ||
| self.check_collaborators_members_support() | ||
|
|
||
| role_enum = normalize_role(workspace_role, WorkspaceRole) |
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.
if we want to get rid of repeated code you can try to wrap this up in some decorator which would make sure that arg would be always Enum
This PR adds a small helper function to normalize role inputs so callers can pass either a role Enum instance or a string (case/whitespace-insensitive). Invalid inputs return None, allowing the caller to handle validation consistently.
solves: #265