Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(api): use ID instead of name for GroupLabel & ProjectLabel _id_attr#2811

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
ptalbert wants to merge1 commit intopython-gitlab:main
base:main
Choose a base branch
Loading
fromptalbert:label_id_attr

Conversation

ptalbert
Copy link

Changes

_id_attr is used when comparing RESTObject objects. Labels will be seen as equal when they have the same name even if they are from different groups or projects.

Instead, use the label's ID as the _id_attr so that comparisions will only be equal if they truly represent the same gitlab label object.

Documentation and testing

Please consider whether this PR needs documentation and tests.This is not required, but highly appreciated:

@ptalbert
Copy link
Author

This change makes sense to me but maybe the original behaviour is preferred for some reason.

I stumbled upon this when fetching all the labels for a number of groups and projects. I put them in a set() and ended up with a lot less items than I expected.

@nejch
Copy link
Member

This change makes a lot of sense, thanks@ptalbert. For context, we didn't use to have_repr_attr so this may have some historical reason but I'm not entirely sure at the moment.

I think this might be a breaking change though, as people might be comparing labels for uniqueness, so I also wonder if we should be usingiid rather thanid, assuming it actually exists for labels (will have to check).

@ptalbert
Copy link
Author

https://docs.gitlab.com/ee/api/labels.html only shows anid field, noiid.

I see the functional tests for groups and projects broke. I don't have docker so I cannot immediately run those tests locally. I will try to get something working.

@ptalbert
Copy link
Author

Quite a few objects usename as their_id_attr :/

python-gitlab (main)$ grep -nrB1 '_id_attr = "name"' gitlab/gitlab/v4/objects/branches.py-21-class ProjectBranch(ObjectDeleteMixin, RESTObject):gitlab/v4/objects/branches.py:22:    _id_attr = "name"--gitlab/v4/objects/branches.py-37-class ProjectProtectedBranch(SaveMixin, ObjectDeleteMixin, RESTObject):gitlab/v4/objects/branches.py:38:    _id_attr = "name"--gitlab/v4/objects/container_registry.py-35-class ProjectRegistryTag(ObjectDeleteMixin, RESTObject):gitlab/v4/objects/container_registry.py:36:    _id_attr = "name"--gitlab/v4/objects/environments.py-62-class ProjectProtectedEnvironment(ObjectDeleteMixin, RESTObject):gitlab/v4/objects/environments.py:63:    _id_attr = "name"--gitlab/v4/objects/features.py-19-class Feature(ObjectDeleteMixin, RESTObject):gitlab/v4/objects/features.py:20:    _id_attr = "name"--gitlab/v4/objects/groups.py-430-class GroupSAMLGroupLink(ObjectDeleteMixin, RESTObject):gitlab/v4/objects/groups.py:431:    _id_attr = "name"--gitlab/v4/objects/tags.py-15-class ProjectTag(ObjectDeleteMixin, RESTObject):gitlab/v4/objects/tags.py:16:    _id_attr = "name"--gitlab/v4/objects/tags.py-32-class ProjectProtectedTag(ObjectDeleteMixin, RESTObject):gitlab/v4/objects/tags.py:33:    _id_attr = "name"--gitlab/v4/objects/templates.py-18-class Dockerfile(RESTObject):gitlab/v4/objects/templates.py:19:    _id_attr = "name"--gitlab/v4/objects/templates.py-30-class Gitignore(RESTObject):gitlab/v4/objects/templates.py:31:    _id_attr = "name"--gitlab/v4/objects/templates.py-42-class Gitlabciyml(RESTObject):gitlab/v4/objects/templates.py:43:    _id_attr = "name"--gitlab/v4/objects/labels.py-25-class GroupLabel(SubscribableMixin, SaveMixin, ObjectDeleteMixin, RESTObject):gitlab/v4/objects/labels.py:26:    _id_attr = "name"--gitlab/v4/objects/labels.py-89-):gitlab/v4/objects/labels.py:90:    _id_attr = "name"

And there is a similar issue with ProjectMergeRequest objects. The_id_attr isiid. This means if you compare two MRs from completely different projects they will be seen as equal if they happen to have the same IID value.

Looks like epics and issues also suffer:

python-gitlab (main)$ grep -nr '_id_attr = "iid"' gitlab/gitlab/v4/objects/epics.py:29:    _id_attr = "iid"gitlab/v4/objects/issues.py:117:    _id_attr = "iid"gitlab/v4/objects/merge_requests.py:155:    _id_attr = "iid"

I imagine most of these objects never get compared but you never know, and certainly a Merge Request might. It seems prudent to fix these.

@nejch
Copy link
Member

@ptalbert thanks. In some casesname is intentional, for example when there is noid returned (like for branches). So if there are other cases where this should not be the case, they would need to be analyzed individually.

Theiid vsid is a known issue, one solution would be to change the eq/hash dunder methods (#935).

@nejch
Copy link
Member

@ptalbert I think for others it mostly makes sense to keepname as_id_attr (e.g. see protected environments =>https://docs.gitlab.com/ee/api/protected_environments.html#get-a-single-protected-environment).

But for this one it makes sense probably, we just need to make sure the tests pass and add a breaking change trailer in your commit message so we ensure a major release for this. Let me know if you need any help.

@nejch
Copy link
Member

Hey@ptalbert, just wondering if you were still willing to work on this as it needs a little tweak it seems. Otherwise we can maybe pick it up in#2926.

_id_attr is used when comparing RESTObject objects. Labels will be seenas equal when they have the same name even if they are from differentgroups or projects.Instead, use the label's ID as the _id_attr so that comparisions willonly be equal if they truly represent the same gitlab label object.Signed-off-by: Patrick Talbert <ptalbert@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees

@ptalbertptalbert

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ptalbert@nejch

[8]ページ先頭

©2009-2025 Movatter.jp