Closed samrocketman closed 7 years ago
That specific example's a definite bug due to my lazily coded delete_member() function. I'll issue a fix for it soon. Update functions may have a similar issue...
"Perhaps doing a test for object type before executing anything and provide a meaningful exception." For similar cases, I'll probably add something like this.
Part of the problem stems from confusion of having multiple types of users/members. It's not something I like, but I don't know how to avoid given the current GitLab API. Adding some type checking here and there like you suggested may work well enough. I'm open to any other ideas though.
Thanks for the feedback!
What would have been the proper way to delete a member from a group?
Due in part to the crippled group membership API (no get or update) and the inflexibility of group.delete_member(), there's only one awkward way to do it and that's to use a Member object returned from a call to group.members(). This is obviously a problem, heh. I'll see about allowing delete_member/delete_user/etc. to accept any type of user or member object.
*edit: Just realized you can also use group.find_member() and pass the result to group.delete_member() (or call delete() on the result). This still isn't ideal though since find_member() will potentially have to query the entire list of members to return a single one.
Perhaps modify this function so that when a sub_api
is added you set a value, parent_api
, in the Class. Then when certain functions are called (e.g. delete()
) it checks that the object being passed matches the type (e.g. type User
) and that parent_api
is set to the current Class type self
. This way only children objects can be passed as an argument and if a global User
object is passed parent_api
will either ==
to None
or to GitLab
depending on how you want to set it up.
Committed a workaround for now to ensure the type is correct, but ideally I'd like to make it more flexible and, e.g. allow a gitlab3.User object be passed to gitlab3.Group.delete_member().
I'm closing this bug report as new issues should be reported for delete problems and api protections.
While the dynamically generated objects are fun it's really easy to hurt yourself. For example I did the following....
I deleted my user entirely from GitLab. When I'm developing against an API I usually use the python interactive interpreter to test out the functions as I go along. Can you think of a way to protect against actions like this? I imagine that if I passed a group object to that
delete_member
function it would delete the group as well (and delete all of the projects located within it).Luckily I'm on a test instance doing development but I think it would be useful to think about. I'm still new to your well formed interface python code. I've not seen anybody create classes like that before (though it is pretty cool). I'll try to brainstorm and come up with an acceptable solution as well. Perhaps doing a test for object type before executing anything and provide a meaningful exception.
The reason why I thought to do that with the user object is because in the interactive prompt I saw the following error message...
where the
int
is myuser.id
. I thought to myself, "Oh, it must want a user object!" Sadly I boarded the fail train.:goberserk: