-
Notifications
You must be signed in to change notification settings - Fork 4
[SK-1024] Organization Custom Roles #38
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: main
Are you sure you want to change the base?
Conversation
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.
Please check the comments.
scalekit/role.py
Outdated
|
||
def __init__(self, core_client: CoreClient): | ||
""" | ||
Initializer for Organization Client |
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.
please update this doc string.
scalekit/role.py
Outdated
self, organization_id: str, role_id: str | ||
) -> UpdateOrganizationRoleResponse: | ||
""" | ||
Method to update organization based on given data |
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.
Please update doc string to match method func.
scalekit/role.py
Outdated
self, organization_id: str, role_id: str | ||
) -> GetOrganizationRoleResponse: | ||
""" | ||
Method to update organization based on given data |
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.
Please update doc string and doc params to match method def and func
scalekit/utils/directory.py
Outdated
GROUPS_FIELD_NUMBER: _ClassVar[int] | ||
USER_DETAIL_FIELD_NUMBER: _ClassVar[int] | ||
id: str | None | ||
id: Optional[str] |
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 not change it to older pythonic way. Please revert to original.
scalekit/role.py
Outdated
|
||
|
||
class RoleClient: | ||
"""Class definition for Organization Client""" |
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.
Please update this doc string too, check if there are any other which are not matching function/class definitions.
scalekit/utils/directory.py
Outdated
from google.protobuf.internal import containers as _containers | ||
from typing import ClassVar as _ClassVar, Iterable as _Iterable, Mapping as _Mapping, Optional as _Optional, \ | ||
Union as _Union | ||
Union as _Union, Optional |
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.
please remove unused imports.
No description provided.