#49492 Issue #35 - dsconf automember support
Closed by spichugi. Opened by alisha17.
alisha17/389-ds-base automember  into  master

Download 49492.patch

Bug Description: Add support for managing automember to dsconf

Fix Description: Initial patch which adds AutoMembershipPlugin, AutoMembershipDefinition
and AutoMembershipDefinitions classes to plugins.py and adds tests for checking valid scope,
valid filter and if user is correctly added to the group.

https://pagure.io/lib389/issue/35

Author: Alisha Aneja

Review by: ???

I'm happy with this, code looks good,

@spichugi can you check this also for me.? I think you'll say something about the docstrings ...

It's better to have two lines between test functions. It increases readability

We need to have proper docstrings for every test case.
You can find the guideline for it here: https://fedorapeople.org/~spichugi/html/guidelines.html

 """Test if member is automatically added to the group
:id: 38621a51-03bc-4fba-93ef-7e525df87c5d
:setup: Standalone instance, enabled Auto Membership Plugin
:steps:
     1. Create a user
     2. Assert that the user is member of the group
:expectedresults:
     1. Should be success
     2. Should be success
 """

You can generate :id: with:

python -c 'import uuid; print(uuid.uuid4())'

I think, it's better to get right value in the beginning. Please, use get_attr_vals_bytes

We need to have all new objects documented. Better in the beginning :)
For init part, you can add the following docstring:

"""A single instance of Auto Membership Plugin entry
:param instance: An instance
:type instance: lib389.DirSrv
:param dn: Entry DN
:type dn: str
"""

Here it can be something like this:

   """Get grouping attributes
    :returns: autoMemberGroupingAttr values
    """

For the Python 3 sake, we better get the values while specifying type explicitly. Like get_attr_vals_utf8

"""DSLdapObjects that represents Auto Membership Plugin entry

:param instance: An instance
:type instance: lib389.DirSrv
:param basedn: Base DN for all account entries below
:type basedn: str
"""

Done. Please, check it and correct me if I've missed some points :)

rebased onto 69a015c5a0a0bfd8169ec1cf88510ff7dfa2edf9

1 new commit added

  • Issue #35 - dsconf automember support

Great! I think this is much better. Just need to fix set_scope I think and rebase to master, then we can merge :)

rebased onto cf25c958ec123cf71f78aca51e0a9a1fbd2f8331

rebased onto bcc13d3da64a5cd90b401844b6e2e134ec77624a

rebased onto debe2781a39dd9be839caefdc4ff1b796df2d320

Pull-Request has been merged by firstyear

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2551

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

Metadata