As I mentioned before, please don't conflate tickets/issues and PRs. Issues in pagure are used for traceability and cross-linking to bugzilla. PRs can't be used for this.

Please create an issue for this change and link back to the issue in the commit message using "Fixes/Relates" keywords (see http://www.port389.org/docs/389ds/contributing.html#getting-the-patch-ready and https://pagure.io/389-ds-base/pull-request/50444#comment-89063). This way PR and issue will be automatically cross-linked.

I appreciate fixing this, but it really should be a separate PR linking (using Relates keyword) to #50627.

This test fails on a build from this PR with:

E         ldap.INSUFFICIENT_ACCESS: {'desc': 'Insufficient access', 'info': "Insufficient 'write' privilege to the 'legalName' attribute of entry 'uid=test_nsu
ser,ou=people,dc=example,dc=com'.\n"}

Is this expected?

There are sometimes issues with python paths and not installing correctly if the file exists ... so ensure you have the PYTHONPATH set to the git check of lib389 and it works.

I appreciate fixing this, but it really should be a separate PR linking (using Relates keyword) to #50627.

Yep. I can split this out. It was causing tests to crash for me.

https://pagure.io/389-ds-base/pull-request/50643 <<-- the split out conftest patch

As I mentioned before, please don't conflate tickets/issues and PRs. Issues in pagure are used for traceability and cross-linking to bugzilla. PRs can't be used for this.
Please create an issue for this change and link back to the issue in the commit message using "Fixes/Relates" keywords (see http://www.port389.org/docs/389ds/contributing.html#getting-the-patch-ready and https://pagure.io/389-ds-base/pull-request/50444#comment-89063). This way PR and issue will be automatically cross-linked.

I feel like we're going to need to reconsider this in the future to make it easier to push small fixes etc without a high admin overhead.

But for now: https://pagure.io/389-ds-base/issue/50644

I feel like we're going to need to reconsider this in the future to make it easier to push small fixes etc without a high admin overhead.

I feel we need to discuss this first before making changes in process. Let's move this to the mailing list?

And changing default aci is not a small fix IMHO. Things like this need to have an audit trail.

I'll put something on the ml now then :)

Okay, no one has responded ... or at least objected. So I'll merge this then!

Actually, no one has said "ack" so I'll wait. @mreynolds and @vashirov ??

@firstyear, so @vashirov was asking that you create a ticket so we track this change. Viktor also noted that a test is failing with this PR. So we need to get those things fixed first before acking...

Uhhhh @mreynolds I did make the ticket. https://pagure.io/389-ds-base/issue/50644
The test failing also can't be reproduced by me, I think that's an environment thing ....

Test fails with the perl installer, because new acis are not created.
So either
1. ldap/ldif/template.ldif should be updated.
or
2. test should be xfailed when enable_perl = yes in defaults.inf.

I'd prefer 1. for the consistency. WDYT?

Well, template.ldif isn't updated for the 1.4.0 changes either? So updating it now seems incorrect because it's already inconsistent.

I think the xfail option may be better here because it seems like the least movement/disruption? Most tests add their aci/data as needed anyway, and ignore the default entries.... There is certainly an argument to "test" instances having a different example database setup for testing consistency vs what we ship to users for their out of box experience.

@vashirov marked with skipif flags now for perl.

Review reminder :)

It passes the basic test suite. Now something you need to do now is how I changed how the sample entries are created. Previously the sample entries assumed you used dc=DOMAIN styling. But it failed for base dn's that use "o", "ou" or "cn", etc.

Check out config_001004000.py and you'll see what I did. Should be a minor change to apply the same thing to config_001004002.py.

Thanks!

@mreynolds I'll have a look, is this a change that's in git master and I need to rebase to fix it? Where are the changes is what I'm asking. Thanks!

DW found it :)

You'll need to review again @mreynolds as I did a minor refactor to standardise the suffix_obj into sampleentries, and then have both the configs call that in their work. But it shoulddo what you want :)

Looks great! Ack

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/3696

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.