RSDK-7188: add app wrappers #614
Conversation
purplenicole730
changed the title
src/viam/app/app_client.py
Outdated
| self.created_at = registry_item.created_at.ToDatetime() | ||
| self.updated_at = registry_item.updated_at.ToDatetime() |
There was a problem hiding this comment.
Given that this is the only value we're changing and the ToDatetime method is already provided by proto, I'm inclined to think we don't need this shadow type at all and are fine to just let users interact with the proto object directly.
src/viam/app/app_client.py
Outdated
|
|
||
| :: | ||
|
|
||
| id = await cloud.get_user_id_by_email("youremail@email.com")`) |
There was a problem hiding this comment.
(minor) probably should get rid of the "`)" at the end
src/viam/app/app_client.py
Outdated
| Args: | ||
| org_id (Optional[str]): The ID of the organization. Defaults to None. | ||
| """ | ||
| org_id = org_id if org_id is not None else await self._get_organization_id() |
There was a problem hiding this comment.
Hmm... this is definitely correct given the state of main now, but it won't be correct once v0.21.0 is merged (we've gotten rid of the _get_organization_id() method and will require users to track and pass org ids themselves).
I think it's a bit unfortunate for us to introduce new methods that rely on _get_organization_id now only to have to change them so soon. (we just released v0.20.0 so the next release will be changing this). Sorry to ask for the refactor, but can we have this fork off of v0.21.0 instead and ask people to pass org_id to the appropriate methods directly?
src/viam/app/app_client.py
Outdated
| resource_type (Union[Literal["organization"], Literal["location"], Literal["robot"]]): Type of the resource the role is added | ||
| to. Must match `resource_id`. |
There was a problem hiding this comment.
Nice catch here on the mistaken comment here 🎉! But, I think we'd want to update the resource_type comment to say that the role is being "removed from" a resource rather than "added to"?
There was a problem hiding this comment.
Yeah, that makes sense. I was just thinking about the fact that it was added to the resource_type previously, but we are removing it so "being removed from" makes a lot of sense!
src/viam/app/app_client.py
Outdated
| old_identity_id (str): ID of the entity the role belongs to (e.g., a user ID). | ||
| old_role (Union[Literal["owner"], Literal["operator"]]): The role to be changed. | ||
| old_resource_type (Union[Literal["organization"], Literal["location"], Literal["robot"]]): Type of the resource the role is | ||
| added to. Must match `resource_id`. |
There was a problem hiding this comment.
(nit) s/resource_id/old_resource_id
src/viam/app/app_client.py
Outdated
| new_identity_id (str): New ID of the entity the role blongs to (e.g., a user ID). | ||
| new_role (Union[Literal["owner"], Literal["operator"]]): The new role. | ||
| new_resource_type (Union[Literal["organization"], Literal["location"], Literal["robot"]]): Type of the resource to add role to. | ||
| Must match `resource_id`. |
There was a problem hiding this comment.
(nit) s/resource_id/new_resource_id
|
|
||
| async def list_organization_members(self, org_id: str) -> Tuple[List[OrganizationMember], List[OrganizationInvite]]: | ||
| """List the members and invites of the currently authed-to organization. | ||
|
|
||
| :: | ||
|
|
||
| member_list, invite_list = await cloud.list_organization_members() | ||
| member_list, invite_list = await cloud.list_organization_members("org-id") |
There was a problem hiding this comment.
thank you for updating all these docstring comments! I appreciate you catching and fixing 🎉
No description provided.