Too many parameters in DELETE action for a project #107
Description
As I'm writing a tool that uses python-gitlab, while writing tests I ran into an issue. When I used the API, to delete a project, I did:
try:
repository = self.gl.projects.get('{}/{}'.format(user, repo))
if repository:
result = repository.delete()
if not repository or not result:
# handle missing repo or bad result here
except GitlabGetError as err:
# handle gitlab errors here
which is working fine… Except for tests, as I'm using Betamax to record queries made to the API, the URL used for deletion contains ALL the attributes of the Project class:
https://gitlab.com/api/v3/projects/1010971?name_with_namespace=Guyzmo+%2F+foobar&builds_enabled=True&forks_count=0&events=%3Cgitlab.objects.ProjectEventManager+object+at+0x7fb5f80f5940%3E&namespace=%3Cclass+%27gitlab.objects.Group%27%3E+%3D%3E+%7B%27description%27%3A+%27%27%2C+%27id%27%3A+540439%2C+%27path%27%3A+%27guyzmo%27%2C+%27membership_lock%27%3A+False%2C+%27gitlab%27%3A+%3Cgitlab.Gitlab+object+at+0x7fb5f90ba470%3E%2C+%27last_ldap_sync_at%27%3A+None%2C+%27updated_at%27%3A+%272016-03-21T12%3A53%3A00.139Z%27%2C+%27members%27%3A+%3Cgitlab.objects.GroupMemberManager+object+at+0x7fb5f80f57b8%3E%2C+%27name%27%3A+%27guyzmo%27%2C+%27avatar%27%3A+None%2C+%27visibility_level%27%3A+20%2C+%27_from_api%27%3A+False%2C+%27created_at%27%3A+%272016-03-21T12%3A53%3A00.139Z%27%2C+%27owner_id%27%3A+459552%2C+%27share_with_group_lock%27%3A+False%7D&last_activity_at=2016-03-29T12%3A52%3A39.919Z&creator_id=459552&hooks=%3Cgitlab.objects.ProjectHookManager+object+at+0x7fb5f80f59e8%3E&created_at=2016-03-29T12%3A52%3A37.142Z&issues_enabled=True&milestones=%3Cgitlab.objects.ProjectMilestoneManager+object+at+0x7fb5f80f5b38%3E&builds=%3Cgitlab.objects.ProjectBuildManager+object+at+0x7fb5f80f5898%3E&http_url_to_repo=https%3A%2F%2Fgitlab.com%2Fguyzmo%2Ffoobar.git&keys=%3Cgitlab.objects.ProjectKeyManager+object+at+0x7fb5f80f5a20%3E&path=foobar&gitlab=%3Cgitlab.Gitlab+object+at+0x7fb5f90ba470%3E&public_builds=True&commitstatuses=%3Cgitlab.objects.ProjectCommitStatusManager+object+at+0x7fb5f80f5908%3E&forks=%3Cgitlab.objects.ProjectForkManager+object+at+0x7fb5f80f59b0%3E&wiki_enabled=True&visibility_level=0&name=foobar&branches=%3Cgitlab.objects.ProjectBranchManager+object+at+0x7fb5f80f5860%3E&tags=%3Cgitlab.objects.ProjectTagManager+object+at+0x7fb5f80f5be0%3E&issues=%3Cgitlab.objects.ProjectIssueManager+object+at+0x7fb5f80f5a58%3E&id=1010971&triggers=%3Cgitlab.objects.ProjectTriggerManager+object+at+0x7fb5f80f5c18%3E&snippets_enabled=False&commits=%3Cgitlab.objects.ProjectCommitManager+object+at+0x7fb5f80f58d0%3E&mergerequests=%3Cgitlab.objects.ProjectMergeRequestManager+object+at+0x7fb5f80f5b00%3E&labels=%3Cgitlab.objects.ProjectLabelManager+object+at+0x7fb5f80f5a90%3E&_from_api=True&files=%3Cgitlab.objects.ProjectFileManager+object+at+0x7fb5f80f5978%3E&star_count=0&variables=%3Cgitlab.objects.ProjectVariableManager+object+at+0x7fb5f80f5c50%3E¬es=%3Cgitlab.objects.ProjectNoteManager+object+at+0x7fb5f80f5b70%3E&owner=%3Cclass+%27gitlab.objects.User%27%3E+%3D%3E+%7B%27username%27%3A+%27guyzmo%27%2C+%27id%27%3A+459552%2C+%27avatar_url%27%3A+%27https%3A%2F%2Fsecure.gravatar.com%2Favatar%2F917dc55c63895af9953df7d798cdd5f8%3Fs%3D80%26d%3Didenticon%27%2C+%27_from_api%27%3A+False%2C+%27name%27%3A+%27Guyzmo%27%2C+%27state%27%3A+%27active%27%2C+%27gitlab%27%3A+%3Cgitlab.Gitlab+object+at+0x7fb5f90ba470%3E%2C+%27web_url%27%3A+%27https%3A%2F%2Fgitlab.com%2Fu%2Fguyzmo%27%2C+%27keys%27%3A+%3Cgitlab.objects.UserKeyManager+object+at+0x7fb5f80f5828%3E%7D&runners_token=4v7LEmxoqUWouMQQsX5r&web_url=https%3A%2F%2Fgitlab.com%2Fguyzmo%2Ffoobar&merge_requests_enabled=True&members=%3Cgitlab.objects.ProjectMemberManager+object+at+0x7fb5f80f5ac8%3E&path_with_namespace=guyzmo%2Ffoobar&public=False&ssh_url_to_repo=git%40gitlab.com%3Aguyzmo%2Ffoobar.git&shared_runners_enabled=True&archived=False&permissions=project_access&permissions=group_access&snippets=%3Cgitlab.objects.ProjectSnippetManager+object+at+0x7fb5f80f5ba8%3E&open_issues_count=0
which, besides being ugly, messy, and containing details that the API cannot understand (like path to instances names), it really makes impossible to use cassettes, because as dict are not ordered, it changes at every call:
https://gitlab.com/api/v3/projects/1010971?labels=%3Cgitlab.objects.ProjectLabelManager+object+at+0x7fc9378ba4a8%3E&name=foobar&name_with_namespace=Guyzmo+%2F+foobar&builds=%3Cgitlab.objects.ProjectBuildManager+object+at+0x7fc9378ba2b0%3E&archived=False¬es=%3Cgitlab.objects.ProjectNoteManager+object+at+0x7fc9378ba588%3E&http_url_to_repo=https%3A%2F%2Fgitlab.com%2Fguyzmo%2Ffoobar.git&keys=%3Cgitlab.objects.ProjectKeyManager+object+at+0x7fc9378ba438%3E&events=%3Cgitlab.objects.ProjectEventManager+object+at+0x7fc9378ba358%3E&commitstatuses=%3Cgitlab.objects.ProjectCommitStatusManager+object+at+0x7fc9378ba320%3E&star_count=0&open_issues_count=0&branches=%3Cgitlab.objects.ProjectBranchManager+object+at+0x7fc9378ba278%3E&milestones=%3Cgitlab.objects.ProjectMilestoneManager+object+at+0x7fc9378ba550%3E&members=%3Cgitlab.objects.ProjectMemberManager+object+at+0x7fc9378ba4e0%3E&gitlab=%3Cgitlab.Gitlab+object+at+0x7fc9385b4550%3E&path=foobar&forks=%3Cgitlab.objects.ProjectForkManager+object+at+0x7fc9378ba3c8%3E&runners_token=4v7LEmxoqUWouMQQsX5r&snippets_enabled=False&forks_count=0&visibility_level=0&owner=%3Cclass+%27gitlab.objects.User%27%3E+%3D%3E+%7B%27keys%27%3A+%3Cgitlab.objects.UserKeyManager+object+at+0x7fc9378ba240%3E%2C+%27_from_api%27%3A+False%2C+%27gitlab%27%3A+%3Cgitlab.Gitlab+object+at+0x7fc9385b4550%3E%2C+%27name%27%3A+%27Guyzmo%27%2C+%27avatar_url%27%3A+%27https%3A%2F%2Fsecure.gravatar.com%2Favatar%2F917dc55c63895af9953df7d798cdd5f8%3Fs%3D80%26d%3Didenticon%27%2C+%27id%27%3A+459552%2C+%27web_url%27%3A+%27https%3A%2F%2Fgitlab.com%2Fu%2Fguyzmo%27%2C+%27state%27%3A+%27active%27%2C+%27username%27%3A+%27guyzmo%27%7D&issues=%3Cgitlab.objects.ProjectIssueManager+object+at+0x7fc9378ba470%3E&last_activity_at=2016-03-29T12%3A52%3A39.919Z&public_builds=True&variables=%3Cgitlab.objects.ProjectVariableManager+object+at+0x7fc9378ba668%3E&id=1010971&web_url=https%3A%2F%2Fgitlab.com%2Fguyzmo%2Ffoobar&tags=%3Cgitlab.objects.ProjectTagManager+object+at+0x7fc9378ba5f8%3E&permissions=project_access&permissions=group_access&path_with_namespace=guyzmo%2Ffoobar&issues_enabled=True&builds_enabled=True&namespace=%3Cclass+%27gitlab.objects.Group%27%3E+%3D%3E+%7B%27description%27%3A+%27%27%2C+%27gitlab%27%3A+%3Cgitlab.Gitlab+object+at+0x7fc9385b4550%3E%2C+%27name%27%3A+%27guyzmo%27%2C+%27path%27%3A+%27guyzmo%27%2C+%27id%27%3A+540439%2C+%27visibility_level%27%3A+20%2C+%27members%27%3A+%3Cgitlab.objects.GroupMemberManager+object+at+0x7fc9378ba1d0%3E%2C+%27_from_api%27%3A+False%2C+%27last_ldap_sync_at%27%3A+None%2C+%27membership_lock%27%3A+False%2C+%27created_at%27%3A+%272016-03-21T12%3A53%3A00.139Z%27%2C+%27avatar%27%3A+None%2C+%27share_with_group_lock%27%3A+False%2C+%27owner_id%27%3A+459552%2C+%27updated_at%27%3A+%272016-03-21T12%3A53%3A00.139Z%27%7D&created_at=2016-03-29T12%3A52%3A37.142Z&shared_runners_enabled=True&snippets=%3Cgitlab.objects.ProjectSnippetManager+object+at+0x7fc9378ba5c0%3E&_from_api=True&commits=%3Cgitlab.objects.ProjectCommitManager+object+at+0x7fc9378ba2e8%3E&wiki_enabled=True&files=%3Cgitlab.objects.ProjectFileManager+object+at+0x7fc9378ba390%3E&public=False&triggers=%3Cgitlab.objects.ProjectTriggerManager+object+at+0x7fc9378ba630%3E&hooks=%3Cgitlab.objects.ProjectHookManager+object+at+0x7fc9378ba400%3E&creator_id=459552&merge_requests_enabled=True&mergerequests=%3Cgitlab.objects.ProjectMergeRequestManager+object+at+0x7fc9378ba518%3E&ssh_url_to_repo=git%40gitlab.com%3Aguyzmo%2Ffoobar.git
When I looked at the source code, I understood why it does that. The Gitlab.delete() code contains the following snippet:
if inspect.isclass(obj):
if not issubclass(obj, GitlabObject):
raise GitlabError("Invalid class: %s" % obj)
params = {}
params[obj.idAttr] = id
else:
params = obj.__dict__.copy()
params.update(kwargs)
so if you give to delete() an instance, it will copy all the attributes from the instance as parameters, making the full messy URL query above. But if you give it a class, and an id, then it gets to send (almost) the right URL:
try:
repository = self.gl.projects.get('{}/{}'.format(user, repo))
if repository:
result = self.gl.delete(repository.__class__, repository.id)
if not repository or not result:
# handle missing repo or bad result here
except GitlabGetError as err:
# handle gitlab errors here
which calls:
https://gitlab.com/api/v3/projects/1010971?id=1010971
here, the id parameter is not useful, but harmless.
I also found out that after using the delete method as I was doing, I'm having gitlab getting broken for a while, giving me 500 error on most pages. I'm not sure whether this is related, but it's quite possible (I opened up issues on gitlab.com's issues tracking page).
So I believe that the GitlabObject.delete() and Gitlab.delete() methods scheme is broken, as it should never give way too many params, many of them the API cannot understand. But I'm not sure how to fix this, and be sure I wouldn't be breaking something elsewhere.
Metadata
Metadata
Assignees
Labels
No labels