#49917 db2ldif and ldif2db methods in lib389 needs to have default values for parameters
Closed: wontfix 2 years ago by mreynolds. Opened 3 years ago by amsharma.

Issue Description

db2ldif and ldif2db methods in lib389 needs to have default values for parameters.

File - https://pagure.io/389-ds-base/blob/master/f/src/lib389/lib389/__init__.py

Example - https://pagure.io/389-ds-base/blob/master/f/src/lib389/lib389/__init__.py#_2740 should be changed to something like def ldif2db(self, bename, suffixes , excludeSuffixes = None, encrypt=False, import_file):
and same with db2ldif.

And this change calls for the change in all the files which are currently using this method.


Hmmmmm, why do they need default values? The point is you have to request the suffix you want because in "not all cases" do we actually know the required suffix. I'm assuming you want this setup as a "laziness" hack for when you call ldif2db/db2ldif when writing tests, but we have to remember that lib389 is an API that people can use. We can't fill it with our convinences as we like, because it could cause issues for future users (like freeipa, and our own CLI tools).

As a result, I think this request is not something I would be comfortable with in the library. Sorry :(

Metadata Update from @firstyear:
- Custom field component adjusted to None
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Custom field type adjusted to None
- Custom field version adjusted to None

3 years ago

Hmmmmm, why do they need default values? The point is you have to request the suffix you want because in "not all cases" do we actually know the required suffix. I'm assuming you want this setup as a "laziness" hack for when you call ldif2db/db2ldif when writing tests, but we have to remember that lib389 is an API that people can use. We can't fill it with our convinences as we like, because it could cause issues for future users (like freeipa, and our own CLI tools).
As a result, I think this request is not something I would be comfortable with in the library. Sorry :(

I think it is a legit request. It is a wrapper around the existing tool and we should follow its logic. bename or suffixes should be specified and we can check that one of the values is not None in the function. The rest of the arguments is optional in the original tool.

Hmmmmm, why do they need default values? The point is you have to request the suffix you want because in "not all cases" do we actually know the required suffix. I'm assuming you want this setup as a "laziness" hack for when you call ldif2db/db2ldif when writing tests, but we have to remember that lib389 is an API that people can use. We can't fill it with our convinences as we like, because it could cause issues for future users (like freeipa, and our own CLI tools).
As a result, I think this request is not something I would be comfortable with in the library. Sorry :(

May be misguided with suffix value ( I fixed that in my original request). It is specially for the parameters which are not used on regular basis like this piece of code -
topo.standalone.stop()
if not topo.standalone.db2ldif(bename=DEFAULT_BENAME, suffixes=(DEFAULT_SUFFIX,),
excludeSuffixes=None, encrypt=False, repl_data=None, outputfile=export_ldif)
:
log.fatal('Failed to run offline db2ldif')
assert False
topo.standalone.start()
It will be nice to have None for these parameters and it can be still used for any use case without any issue there.

Metadata Update from @mreynolds:
- Issue set to the milestone: 1.4.0

3 years ago

Metadata Update from @mreynolds:
- Issue close_status updated to: wontfix
- Issue status updated to: Closed (was: Open)

2 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/2976

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.

Login to comment on this ticket.

Metadata