The delete agreement function of agreement.py module doesn't cover all usage varieties and should be rewritten.
Now it requires suffix and replica(DirSrv object of the server that the agreement points to). By some reasons, this replica object may be missing, then we couldn't delete the agreement.
I propose to change arguments of the delete agreement function to:
With that, we can clearly define the required agreement.
If anybody has some additional ideas, please share them.
Also, we don't have test coverage for that function. It should be added to tests/agreement_test.py.
attachment 0001-Ticket-48360-Refactor-the-delete-agreement-function.patch
I'm happy with the code here for the deletion: I think this is much more specific and a good improvement.
given that the cn of a replication agreement has to be unique also, could we also delete by agreement name?
If you want to add deletion by agreement name, I'd be happy with that, if not your current patch has my ack.
attachment 0001-Ticket-48360-Refactor-the-delete-agreement-function-2.patch
William, sure, no problem.
Feature was added, please, test and review.
The new feature looks good!
With the test case, because we only delete one test aren't we only testing the agmtdn delete code path? We need two tests, one for agmtdn, and one for consumer_host / port. Sorry to be a pain!
Replying to [comment:4 firstyear]:
The new feature looks good! With the test case, because we only delete one test aren't we only testing the agmtdn delete code path? We need two tests, one for agmtdn, and one for consumer_host / port. Sorry to be a pain!
No problem. Thank you. :)
For now, I have other tasks with a higher priority. I've added a "TODO" block, so I can come back to it when the time will come.
{{{ 187 # TODO: add a few test cases for different sets of arguments 188 # using fixtures and parameters. }}}
attachment 0001-Ticket-48360-Refactor-the-delete-agreement-function-2.2.patch
Just so long as you do come back to it! :)
Ack from me.
To ssh://git.fedorahosted.org/git/389/lib389.git
252e0c1..0b9729b master -> master commit 0b9729bacf5a8a6529559d81044fbb5973cf9b84 Author: Simon Pichugin spichugi@redhat.com Date: Thu Nov 26 16:55:52 2015 +0100
Milestone lib389 1.0 deleted
Metadata Update from @spichugi: - Issue assigned to spichugi
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/1691
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.