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

Appearance settings

Conversation

@costela
Copy link

@costela costela commented Sep 16, 2016

This PR add support for group.ldaplinks.create() and group.ldaplinks.delete().

It's unclear to me what the best way to implement .list() would be. Since the API returns the LDAP information embedded in the group, I see two options (maybe I'm missing something obvious):

  • hide the ldap_group_links attribute behind a manager and add a list() method that fetches the group again, ignoring all info but the LDAP links
  • keep the .ldap_group_links and the .ldaplinks manager separate, but somehow update the .ldap_group_links list when we perform changes (currently the list remains out-dated whenever we perform changes, which obviously sucks)

Not sure how any of these could be implemented... :)

@gpocentek
Copy link
Contributor

Thank you for your work on this patch.

It looks OK apart from a few syntax changes required. But I'm not comfortable merging it because this is a gitlab EE feature that I can't test. I'm not sure how I could automate testing and make sure the code works.

@costela
Copy link
Author

costela commented Sep 18, 2016

Oh, I totally forgot this annoying detail :/
I believe the enterprise edition is actually installable without a license (it's user based), so I could ask the Gitlab guys if they could issue a sort of "for automated testing purposes" blanket license. Than you could test against EE. Would you be OK with this? (assuming it's possible...)

@gpocentek
Copy link
Contributor

It's worth a try. I'd be happy to add EE features to the lib. Thanks!

@costela
Copy link
Author

costela commented Sep 19, 2016

Just added a commit that changes the testing to use a fork of your test-python-gitlab docker image using gitlab EE, but I keep getting timeouts. Strangely enough my local docker image seems to work...
Also, the timeouts don't seem to signal a build-failure since the whole functional testing is done inside after_success. Was this intentional?

@gpocentek
Copy link
Contributor

I used after_success to avoid running the functional tests multiple times. Maybe this is not such a good idea. I'll try to find a better way to do it as soon as I can but this might take time. Feel free to change the travis config to test possible updates.

Thank you for your work on this!

@jouve
Copy link
Contributor

jouve commented May 22, 2018

I have this working (create/delete) on 1.4.0:

from gitlab.base import RESTObject, RESTManager
from gitlab.mixins import CreateMixin, DeleteMixin
import gitlab.v4.objects

class GroupLdapGroupLink(RESTObject):
    pass


class GroupLdapGroupLinkManager(CreateMixin, DeleteMixin, RESTManager):
    _path = '/groups/%(group_id)s/ldap_group_links'
    _obj_cls = GroupLdapGroupLink
    _from_parent_attrs = {'group_id': 'id'}
    _create_attrs = (('cn', 'group_access', 'provider'), tuple())

gitlab.v4.objects.GroupLdapGroupLinkManager = GroupLdapGroupLinkManager

gitlab.v4.objects.Group._managers = tuple(
    gitlab.v4.objects.Group._managers + (('ldapgrouplinks', 'GroupLdapGroupLinkManager'), )
)

@gpocentek
Copy link
Contributor

The LDAP group link API endpoint being quite different from the other endpoints I implemented support for this feature as Group methods instead of new classes. See commit d6a61af.

@gpocentek gpocentek closed this Jun 9, 2018
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.