#49022 py3 installer, backends can not have domain created
Closed: wontfix None Opened 7 years ago by firstyear.

When we attempt to add:

dn: dc=example,dc=com
changetype: add
objectClass: top
objectClass: domain
dc: example

To a new backend this error is retrieved.

adding new entry "dc=example,dc=com"
ldap_add: No such object (32)

The trace log of this is:

[28/Oct/2016:17:06:35.093087305 +1000] - DEBUG - pagedresults_cleanup - =>
[28/Oct/2016:17:06:35.319694712 +1000] - DEBUG - pagedresults_cleanup - <= 0
[28/Oct/2016:17:06:35.340990448 +1000] - DEBUG - ids_sasl_server_new - => (ldapkdc.example.com)
[28/Oct/2016:17:06:35.362610873 +1000] - DEBUG - ids_sasl_getopt - plugin= option=log_level
[28/Oct/2016:17:06:35.384147844 +1000] - DEBUG - ids_sasl_getopt - plugin= option=auto_transition
[28/Oct/2016:17:06:35.405560295 +1000] - DEBUG - ids_sasl_getopt - plugin= option=mech_list
[28/Oct/2016:17:06:35.427089518 +1000] - DEBUG - ids_sasl_server_new - <=
[28/Oct/2016:17:06:35.448886978 +1000] - DEBUG - add_work_q - =>
[28/Oct/2016:17:06:35.470357441 +1000] - DEBUG - get_work_q - =>
[28/Oct/2016:17:06:35.491878875 +1000] - DEBUG - pagedresults_in_use_nolock - =>
[28/Oct/2016:17:06:35.513213453 +1000] - DEBUG - pagedresults_in_use_nolock - <= 0
[28/Oct/2016:17:06:35.534839566 +1000] - DEBUG - do_bind - =>
[28/Oct/2016:17:06:35.556453141 +1000] - DEBUG - do_bind - BIND dn="cn=Directory Manager" method=128 version=3
[28/Oct/2016:17:06:35.578047368 +1000] - DEBUG - get_ldapmessage_controls_ext - => get_ldapmessage_controls
[28/Oct/2016:17:06:35.599679684 +1000] - DEBUG - get_ldapmessage_controls_ext - <= no controls
[28/Oct/2016:17:06:35.621201992 +1000] - DEBUG - slapi_control_present - => (looking for 2.16.840.1.113730.3.4.16)
[28/Oct/2016:17:06:35.642842561 +1000] - DEBUG - slapi_control_present - <= 0 (NO CONTROLS)
[28/Oct/2016:17:06:35.664470244 +1000] - DEBUG - do_bind - version 3 method 128 dn cn=Directory Manager
[28/Oct/2016:17:06:35.686053204 +1000] - DEBUG - slapi_pw_find value - => "password"
[28/Oct/2016:17:06:35.707792029 +1000] - DEBUG - <= slapi_pw_find matched "%s" using scheme "%s"
 - 3+K6v+ongoRc8cUX3A4IkSMcJGypv9SoTEChCLbpqb0zl9nobUhOal/4xVTXuZkFAbpiSPbS44NSDPEIq6h4Uw3K/pTciEwE[28/Oct/2016:17:06:35.729387231 +1000] - DEBUG - => %s conn=0x%x, entry=0x%x
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.751102124 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.772740821 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.794687735 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.816690314 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.838387522 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.860017450 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.881757926 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.903179847 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.924668159 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.946142231 +1000] - DEBUG - <= %s returning status %d
 - reslimit_update_from_entry()[28/Oct/2016:17:06:35.967849528 +1000] - DEBUG - send_ldap_result_ext - => 0::
[28/Oct/2016:17:06:35.989320547 +1000] - DEBUG - slapi_control_present - => (looking for 1.3.6.1.1.13.1)
[28/Oct/2016:17:06:36.010835616 +1000] - DEBUG - slapi_control_present - <= 0 (NO CONTROLS)
[28/Oct/2016:17:06:36.032324065 +1000] - DEBUG - slapi_control_present - => (looking for 1.3.6.1.1.13.2)
[28/Oct/2016:17:06:36.053828307 +1000] - DEBUG - slapi_control_present - <= 0 (NO CONTROLS)
[28/Oct/2016:17:06:36.076073834 +1000] - DEBUG - send_ldap_result_ext - <= 0
[28/Oct/2016:17:06:36.097883282 +1000] - DEBUG - add_work_q - =>
[28/Oct/2016:17:06:36.119375651 +1000] - DEBUG - get_work_q - =>
[28/Oct/2016:17:06:36.141265184 +1000] - DEBUG - do_add - ==>
[28/Oct/2016:17:06:36.162754231 +1000] - DEBUG - get_ldapmessage_controls_ext - => get_ldapmessage_controls
[28/Oct/2016:17:06:36.184246068 +1000] - DEBUG - get_ldapmessage_controls_ext - <= no controls
[28/Oct/2016:17:06:36.206425035 +1000] - DEBUG - slapi_control_present - => (looking for 2.16.840.1.113730.3.4.12)
[28/Oct/2016:17:06:36.227940931 +1000] - DEBUG - slapi_control_present - <= 0 (NO CONTROLS)
[28/Oct/2016:17:06:36.249366749 +1000] - DEBUG - slapi_control_present - => (looking for 2.16.840.1.113730.3.4.18)
[28/Oct/2016:17:06:36.270703706 +1000] - DEBUG - slapi_control_present - <= 0 (NO CONTROLS)
[28/Oct/2016:17:06:36.292011462 +1000] - DEBUG - add_created_attrs - ==>
[28/Oct/2016:17:06:36.313323384 +1000] - DEBUG - plugin_call_func - Calling plugin 'ACL preoperation' #1 type 407
[28/Oct/2016:17:06:36.334805126 +1000] - DEBUG - slapi_control_present - => (looking for 2.16.840.1.113730.3.4.12)
[28/Oct/2016:17:06:36.356928107 +1000] - DEBUG - slapi_control_present - <= 0 (NO CONTROLS)
[28/Oct/2016:17:06:36.378478772 +1000] - DEBUG - slapi_control_present - => (looking for 2.16.840.1.113730.3.4.18)
[28/Oct/2016:17:06:36.399958984 +1000] - DEBUG - slapi_control_present - <= 0 (NO CONTROLS)
[28/Oct/2016:17:06:36.421962406 +1000] - DEBUG - plugin_call_func - Calling plugin 'ro_replica' #4 type 407
[28/Oct/2016:17:06:36.443674613 +1000] - DEBUG - defbackend_default - <==
[28/Oct/2016:17:06:36.465019343 +1000] - DEBUG - send_ldap_result_ext - => 32::
[28/Oct/2016:17:06:36.486566060 +1000] - DEBUG - send_ldap_result_ext - <= 32
[28/Oct/2016:17:06:36.507417733 +1000] - DEBUG - add_work_q - =>
[28/Oct/2016:17:06:36.528803406 +1000] - DEBUG - get_work_q - =>
[28/Oct/2016:17:06:36.550161129 +1000] - DEBUG - plugin_call_func - Calling plugin 'Class of Service postoperation plugin' #0 type 507
[28/Oct/2016:17:06:36.572106337 +1000] - DEBUG - cos-plugin - --> cos_post_op
[28/Oct/2016:17:06:36.593336368 +1000] - DEBUG - cos-plugin - --> cos_cache_change_notify
[28/Oct/2016:17:06:36.614547551 +1000] - DEBUG - cos-plugin - <-- cos_cache_change_notify
[28/Oct/2016:17:06:36.635847677 +1000] - DEBUG - cos-plugin - <-- cos_post_op
[28/Oct/2016:17:06:36.657495580 +1000] - DEBUG - do_unbind - =>
[28/Oct/2016:17:06:36.678682580 +1000] - DEBUG - get_ldapmessage_controls_ext - => get_ldapmessage_controls
[28/Oct/2016:17:06:36.700250880 +1000] - DEBUG - get_ldapmessage_controls_ext - <= no controls
[28/Oct/2016:17:06:36.722115105 +1000] - DEBUG - defbackend_noop - <==
[28/Oct/2016:17:06:36.753126152 +1000] - DEBUG - pagedresults_reset_timedout - =>
[28/Oct/2016:17:06:36.774316176 +1000] - DEBUG - pagedresults_reset_timedout - <=
[28/Oct/2016:17:06:37.046144115 +1000] - DEBUG - => %s conn=0x%x, entry=0x%x
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.067634232 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.089224072 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.110627765 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.132071098 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.153415917 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.174909898 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.196355157 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.217684813 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.239075555 +1000] - DEBUG - %s: setting limit for handle %d (based on %s)
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.260534967 +1000] - DEBUG - <= %s returning status %d
 - reslimit_update_from_entry()[28/Oct/2016:17:06:37.282131990 +1000] - DEBUG - pagedresults_cleanup - =>
[28/Oct/2016:17:06:37.303559858 +1000] - DEBUG - pagedresults_cleanup - <= 0
[28/Oct/2016:17:06:41.011553984 +1000] - DEBUG - log__needrotation - LOGINFO:End of Log because size exceeded(Max:100 bytes) (Is:1349 bytes)
[28/Oct/2016:17:06:41.033474482 +1000] - DEBUG - log__delete_access_logfile - Removed file:/opt/dirsrv/var/log/dirsrv/slapd-standalone/access.20161028-165819 because of (exceeded maximum log disk space)

Looks good to me. I've run dirsrvtests with Python 2.7. With and without the patch, and the behavior was the same.
And I'll check lib389 as soon as I'll managed to install pyldap with Python 3 properly... It is still not so smooth and I have some problems on my VM.

Thanks, now it passes for Python 2 in the lib389 tests (I guess dirsrvtests just doesn't use all lib389 features right now).

One thing... I don't understand why have you reduced functionality at the backend_test?
Before that it checks for an exception info, not only for UNWILLING_TO_PERFORM. And it uses pytest.raises feature, so it is more pytest way to deal with things. ;)

{{{
577 log.info("Backend name differs -> UNWILLING_TO_PERFORM")
578 - with pytest.raises(ldap.UNWILLING_TO_PERFORM) as excinfo:
579 - topology.standalone.backend.delete(suffix=NEW_SUFFIX_1,
580 - bename='dummydb')
581 - assert 'Backend name specified (dummydb) differs from' in \
582 - str(excinfo.value)
583 + try:
584 + topology.standalone.backend.delete(suffix=NEW_SUFFIX_1, bename='dummydb')
585 + assert(False)
586 + except ldap.UNWILLING_TO_PERFORM:
587 + pass
588 topology.standalone.backend.delete(suffix=NEW_SUFFIX_1)

}}}

Another thing... I can't manage to run it properly with the last pyldap version https://github.com/pyldap/pyldap/ and Python 3, not only your tests, but many of them.

There is numerous issues with string, bytes variables and its decoding (some issues, I believe, are inside pyldap itself). If you've managed to run our code on Python 3, can you please send me your setup via email? Or even share it for everyone in the mail-list.

Replying to [comment:3 spichugi]:

Thanks, now it passes for Python 2 in the lib389 tests (I guess dirsrvtests just doesn't use all lib389 features right now).

One thing... I don't understand why have you reduced functionality at the backend_test?

That test also doesn't matter now, as a delete of the backend, guarantees a delete of the mapping tree if it exists. So the test could never be satisfied.

Before that it checks for an exception info, not only for UNWILLING_TO_PERFORM. And it uses pytest.raises feature, so it is more pytest way to deal with things. ;)

{{{
577 log.info("Backend name differs -> UNWILLING_TO_PERFORM")
578 - with pytest.raises(ldap.UNWILLING_TO_PERFORM) as excinfo:
579 - topology.standalone.backend.delete(suffix=NEW_SUFFIX_1,
580 - bename='dummydb')
581 - assert 'Backend name specified (dummydb) differs from' in \
582 - str(excinfo.value)
583 + try:
584 + topology.standalone.backend.delete(suffix=NEW_SUFFIX_1, bename='dummydb')
585 + assert(False)
586 + except ldap.UNWILLING_TO_PERFORM:
587 + pass
588 topology.standalone.backend.delete(suffix=NEW_SUFFIX_1)

}}}

Yeah, pytest.raises may be nicer, but the intent is not clear (not to me at least).

As well, the test is silly. It's looking for a specific error message on the ldap unwilling to perform that is set by lib389 and could change.

Another thing... I can't manage to run it properly with the last pyldap version https://github.com/pyldap/pyldap/ and Python 3, not only your tests, but many of them.

There is numerous issues with string, bytes variables and its decoding (some issues, I believe, are inside pyldap itself). If you've managed to run our code on Python 3, can you please send me your setup via email? Or even share it for everyone in the mail-list.

Whoa boy. I think I'll share this to the mailing list. The short summary is that right now, python 3 is a mess, and it's painful to setup and use.

Replying to [comment:4 firstyear]:

Replying to [comment:3 spichugi]:
That test also doesn't matter now, as a delete of the backend, guarantees a delete of the mapping tree if it exists. So the test could never be satisfied.
Now it is not, but who knows what can be broken tomorrow? If we already have these tests, why not keep them? It is not bad to test expected behavior in more detailed way.

Yeah, pytest.raises may be nicer, but the intent is not clear (not to me at least).
It is pretty self-descriptive and well readable, in my opinion. As long as we use pytest as our tool, we should try to use it's features at maximum, if it brings benefits. This feature reduces number of written lines. Your code has 5 lines and mine has 2 lines (if we don't consider assert).
Also, other tests written in pytest way withing this test case, so it is better to keep it consistent.

As well, the test is silly. It's looking for a specific error message on the ldap unwilling to perform that is set by lib389 and could change.
We test lib389 features with that test, so I think it is better to have more detailed tests and do not reduce the current functionality.
If we change lib389, we run all our tests and change them too, accordingly to the lib389 changes.

Replying to [comment:5 spichugi]:

Replying to [comment:4 firstyear]:

Replying to [comment:3 spichugi]:
That test also doesn't matter now, as a delete of the backend, guarantees a delete of the mapping tree if it exists. So the test could never be satisfied.
Now it is not, but who knows what can be broken tomorrow? If we already have these tests, why not keep them? It is not bad to test expected behavior in more detailed way.

I see where you are coming from but in this case this could not be satisfied. Either the mapping tree doesn't exist and backend can be deleted, or the mapping tree exists and a backend delete removes it too.

The previous test was testing a "lack of behaviour" in lib389. This change is meant to make backend easier to use. To satisfy this test, would mean undoing the mapping tree automatic delete, which would make any code deriving backend delete require the mapping tree check. This means more places for mistakes. In summary, I'm not undoing this.

Yeah, pytest.raises may be nicer, but the intent is not clear (not to me at least).
It is pretty self-descriptive and well readable, in my opinion. As long as we use pytest as our tool, we should try to use it's features at maximum, if it brings benefits. This feature reduces number of written lines. Your code has 5 lines and mine has 2 lines (if we don't consider assert).
Also, other tests written in pytest way withing this test case, so it is better to keep it consistent.

I will revert to this syntax for you.

As well, the test is silly. It's looking for a specific error message on the ldap unwilling to perform that is set by lib389 and could change.
We test lib389 features with that test, so I think it is better to have more detailed tests and do not reduce the current functionality.
If we change lib389, we run all our tests and change them too, accordingly to the lib389 changes.

We should test behaviours, not error message wording. Testing error message wording is a waste of my time, your time, it can break CI and waste our time investigating. I'm not going to revert this, and in fact, will remove more "tests of wording" in the future.

We should spend time making sure we test behaviour of the library is correct instead.

I'm sorry to disagree with you on these points, but I can not agree with some of your points I'm sorry :(

Replying to [comment:6 firstyear]:

We should test behaviours, not error message wording. Testing error message wording is a waste of my time, your time, it can break CI and waste our time investigating. I'm not going to revert this, and in fact, will remove more "tests of wording" in the future.

We should spend time making sure we test behaviour of the library is correct instead.

I'm sorry to disagree with you on these points, but I can not agree with some of your points I'm sorry :(

It is very good point for a discussion about "the way of the project". :)

Let me share my view on that matter.
I think, it is better to have explicit code and behavior either then implicit. For example, we have ldap.UNWILLING_TO_PERFORM error in our issue. It is very common exception. You can find 12 kinds of ldap.UNWILLING_TO_PERFORM with various error messages only within backend.py. But this is not all! We also have a lot of ldap.UNWILLING_TO_PERFORM within DS itself. If some error happens we need to find the source of the problem. It will be much easier and it will save a lot of time (in this situation, Red Hat time and money), if our tests will cover this work for us and provide the information where exactly the test has failed.

Yes, it will require a bit more work for the one who will develop new features within lib389, because he will need to care not only about lib389 module but also about proper testing. And it is part of the process, isn't it? If the guy feels confident about his work and his understanding of the issue, it will require additional 5-10 minutes to adjust tests properly, although it will save possibly hours during the debugging in the future.
I have no problem with writing tests and I will assist you if you want, so you can concentrate and give more of your strength to the developing.

For now, I just ask you do not reduce current functionality. It may damage us in the future.

Replying to [comment:7 spichugi]:

Replying to [comment:6 firstyear]:

We should test behaviours, not error message wording. Testing error message wording is a waste of my time, your time, it can break CI and waste our time investigating. I'm not going to revert this, and in fact, will remove more "tests of wording" in the future.

We should spend time making sure we test behaviour of the library is correct instead.

I'm sorry to disagree with you on these points, but I can not agree with some of your points I'm sorry :(

It is very good point for a discussion about "the way of the project". :)

Certainly: And I myself have been pushing the project in certain directions myself in the last year.

Let me share my view on that matter.
I think, it is better to have explicit code and behavior either then implicit. For example, we have ldap.UNWILLING_TO_PERFORM error in our issue. It is very common exception. You can find 12 kinds of ldap.UNWILLING_TO_PERFORM with various error messages only within backend.py. But this is not all! We also have a lot of ldap.UNWILLING_TO_PERFORM within DS itself. If some error happens we need to find the source of the problem. It will be much easier and it will save a lot of time (in this situation, Red Hat time and money), if our tests will cover this work for us and provide the information where exactly the test has failed.

When the test fails, you get a stack trace that precisely identifies the line of code and why it failed. It's not C, or something else where you just get "UNWILLING_TO_PERFORM". You'll get a traceback that exactly tells you where the error is. You can easily find the source of the problem.

The message is for when lib389 is bundled into a tool, and the exception is caught then displayed "nicely". It's not for determining where a test failed. In this case, the test is asserting that with "some steps" in a known clean configuration, we get the UNWILLING_TO_PERFORM, so the test is working. It would be the absence of the UNWILLING_TO_PERFORM (and the message) that would be the error here, rather than anything else.

Now it could just be a language barrier here, but perhaps you are saying "where" the test fails as in "you want to test that the exception was thrown by this EXACT location in the code, and you use the message to pin point that".

Again, I think the use of the message is a problem. Messages change.

If you want to test the EXACT location in the code, either make specific error messages for every single error, or we need to find a better way, IE by inserting error codes into our exceptions we raise, or something else. Just not the message, because messages change.

Yes, it will require a bit more work for the one who will develop new features within lib389, because he will need to care not only about lib389 module but also about proper testing. And it is part of the process, isn't it? If the guy feels confident about his work and his understanding of the issue, it will require additional 5-10 minutes to adjust tests properly, although it will save possibly hours during the debugging in the future.

But it will add multiple "5 minutes" to fix in the future every time a message changes, or something like that. How many minutes have we already spent on this discussion ;)

Lets not even start on translation, internationalisation. What happens then? Do we need to localise every test then? Or should we just test the behaviour of the exception is correct, rather than the message?

I have no problem with writing tests and I will assist you if you want, so you can concentrate and give more of your strength to the developing.

For now, I just ask you do not reduce current functionality. It may damage us in the future.

From my perspective I am not reducing functionality here. I'm still asserting the an invalid behavior triggers an exception.

I like the way we've discussed it. Thank you.

You have ack from me. :) When we will have Python 3 CI and proper workflow for it, it will be easier to fix all issues altogether. Now, it is a mess.

commit f97cf65dfefc31f8bd041d34665dc4bae44b9603
Writing objects: 100% (18/18), 7.65 KiB | 0 bytes/s, done.
Total 18 (delta 13), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/lib389.git
6ca8cd9..f97cf65 master -> master

Metadata Update from @firstyear:
- Issue assigned to firstyear
- Issue set to the milestone: lib389 1.0.2

7 years ago

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 issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2081

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata