Skip to content

Convert response list to single data source for iid requests - #169

Merged
gpocentek merged 1 commit into
python-gitlab:masterfrom
hakkeroid:allow-iid-parameter-to-request-distinct-objects
Oct 23, 2016
Merged

Convert response list to single data source for iid requests#169
gpocentek merged 1 commit into
python-gitlab:masterfrom
hakkeroid:allow-iid-parameter-to-request-distinct-objects

Conversation

@hakkeroid

Copy link
Copy Markdown
Contributor

Solves #160

It is questionable though if the iid check should be located here as iids are specific and not available with all gitlab resources.

@gpocentek

Copy link
Copy Markdown
Contributor

Moving the checks elsewhere will lead to extra complexity. Still I'm not entirely convinced that this is the place for this test, but can't seem to find a better place to handle it.

Could you rebase your patch and I think I'll merge it as is since it clearly fixes the bugs and add support for a nice feature. Thank you.

@hakkeroid
hakkeroid force-pushed the allow-iid-parameter-to-request-distinct-objects branch from 67f9537 to 23b5b6e Compare October 23, 2016 09:39
@hakkeroid

Copy link
Copy Markdown
Contributor Author

No problem. I rebased the branch.

I thought about that. What I could think of was another class attribute like "identifiers" which is a list of possible id's to check. This could be a simple ['id'] on the parent class GitlabObject as a sane default and a ['id', 'iid'] on the respective subclasses.
Then any id relevant tests can expect a list of identifiers to check against. Although when going into that direction a simple list might not be enough because iid itself isn't clearly distinctive. Only in combination with project_id.

Another possibility could be to have a dedicated class for id handling. Usually
that seems to be too extreme but gitlab's different concepts of "identity"
might justify that.

Anyway I am happy to see you are merging it.

@hakkeroid

Copy link
Copy Markdown
Contributor Author

Sorry I just saw that there is indeed GitlabObject.idAttr. That might be an opportunity but as you said; definitely more complex.

@gpocentek

Copy link
Copy Markdown
Contributor

Let's merge your patch, it's simple enough and fixes a corner case related to gitlab's special behavior. If we need to deal with more special cases in the future we'll consider a more solid solution.

Thank you for tackling this :)

@gpocentek
gpocentek merged commit 20fdbe8 into python-gitlab:master Oct 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants