maybe for the cli we call it ldifgen not dbgen?

Besides that one naming comment, it otherwise looks good to me. :) Great to see this nearly done!

Could you please take a look at the tests that use lib389.dbgen? They are failing now...

tests/suites/mapping_tree/referral_during_tot_init_test.py:17:from lib389.dbgen import dbgen
tests/suites/basic/basic_test.py:21:from lib389.dbgen import dbgen
tests/suites/syntax/mr_test.py:5:from lib389.dbgen import dbgen
tests/suites/import/regression_test.py:13:from lib389.dbgen import dbgen
tests/stress/search/simple.py:11:from lib389.dbgen import dbgen

Why not get the props from get_input() here too? I think it shouldn't hurt... Or do I miss something?

It gives Error: [Errno 13] Permission denied: '/tmp/dbgen.ldif' error when /tmp/dbgen.ldif already exists.
Maybe the tool should ask something like '/tmp/dbgen.ldif' already exists. Do you want to overwrite it? Or something like this...

Another a bit odd behaviour that I found is when we specify optional arguments like

dsctl localhost dbgen groups --number 10 --suffix dc=example,dc=com my_group

then dbgen just silently assumes that other arguments should be default ones.

It may be intended but then we shouldn't do that silently, in my opinion...
Or, as an option, we should ask about the non-specified arguments.

maybe for the cli we call it ldifgen not dbgen?

Hahaha, my tool I ported most of this from is call "ldif-gen.py", so yes I will gladly make that change :-)

Another a bit odd behaviour that I found is when we specify optional arguments like
dsctl localhost dbgen groups --number 10 --suffix dc=example,dc=com my_group

then dbgen just silently assumes that other arguments should be default ones.
It may be intended but then we shouldn't do that silently, in my opinion...
Or, as an option, we should ask about the non-specified arguments.

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.

Maybe we can print the defaults that were used? Just one simple line.
It will be more explicit for the user. Which never hurts.

And if the user doesn't like the options he can regenerate the thing right away (basically, in the end, it is for speeding up the process)

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.

Maybe we can print the defaults that were used? Just one simple line.
It will be more explicit for the user. Which never hurts.

Isn't that what "--help" is for? :-)

And if the user doesn't like the options he can regenerate the thing right away (basically, in the end, it is for speeding up the process)

Again isn't this what the interactive mode is for? The non-interactive mode should just run and assume you know what you want to do. I do not want to make the non-interactive mode interactive.

So... I'm not rejecting what you are saying, but I don't think I am visualizing what you want it to look like. Maybe you could write up some mock terminal output, or something, and show me how you think it should it behave? Thanks!!

Fixed all of other issues in the meantime:

• Improved LDIF file validation
• Fixed CI tests
• Fixed nested LDIF entries (nsContainer -> organizationalUnit)

Not sure how I feel about this request. You should not have to specify 10 options if you just want to quickly create a basic group with commonly used attributes. I'm trying to make the tool really easy/simple and fast. So, if you want to tweak the default LDIF you can use the other options. Or, if you want that fine grained control you could use the interactive mode which asks for everything.
Maybe we can print the defaults that were used? Just one simple line.
It will be more explicit for the user. Which never hurts.

Isn't that what "--help" is for? :-)

Okay:) I was thinking more and I think it's okay but we should make sure that the --help has all of the defaults in the text.
For example, we can run like this:

 dsctl localhost ldifgen groups --suffix dc=example,dc=com my_group  
 Writing LDIF file /tmp/dbgen.ldif ...
 Successfully created LDIF file: /tmp/dbgen.ldif

But the --help doesn't have a default for the number (and there are more cases like that)

 dsctl localhost ldifgen groups --suffix dc=example,dc=com my_group -h
 ...
 --number NUMBER       The number of groups to create.
 ...

Also, I am a bit confused here:

dsctl localhost ldifgen users --suffix dc=example,dc=com
Missing required parameters, switching to Interactive mode ...
Enter the suffix [dc=example,dc=com]:

Because --help doesn't have any mentioning about these required parameters. So I don't know what I missed (if I want a non-interactive mode)

dsctl localhost ldifgen users -h
usage: dsctl [instance] ldifgen users [-h] [--number NUMBER] [--suffix SUFFIX]
                                  [--parent PARENT] [--generic]
                                  [--start-idx START_IDX] [--rdn-cn]
                                  [--localize] [--ldif-file LDIF_FILE]
optional arguments:
-h, --help            show this help message and exit
--number NUMBER       The number of users to create.
--suffix SUFFIX       The database suffix where the entries will be created.
...

And if the user doesn't like the options he can regenerate the thing right away (basically, in the end, it is for speeding up the process)

So... I'm not rejecting what you are saying, but I don't think I am visualizing what you want it to look like. Maybe you could write up some mock terminal output, or something, and show me how you think it should it behave? Thanks!!

I thought about something like this:

dsctl localhost ldifgen users  --suffix dc=example,dc=com --number=10
Non-interactive mode is used.
Setting the defaults: parent='ou=people,dc=example,dc=com', rdn-cn='yes', etc.
Writing LDIF file /tmp/dbgen.ldif ...
Successfully created LDIF file: /tmp/dbgen.ldif

If I understand correctly, get_ldif_file_input returns NoneType so args.ldif_file becomes corrupted if we go with the interactive option

It gives Error: [Errno 13] Permission denied: '/tmp/dbgen.ldif' error when /tmp/dbgen.ldif already exists.
Maybe the tool should ask something like '/tmp/dbgen.ldif' already exists. Do you want to overwrite it? Or something like this...

Simple option here, why not write to the ldif dir of the related instance? If we plan to import it to that instance, then this would make the most sense because it then isolates between instances, and means the instance can actually read the ldif dir (a common issue with the import from say /root or /tmp is the dirsrv user of the instance can't read the directory the ldif is in ...)

Sorry to follow up, imagine this case:

instanceA = dbgen
instanceB= dbgen
instanceA.import_from_ldif
instanceb.import_from_ldif

The second dbgen would clobber the first. So if we had parallel tests or anything else it could destroy evidence or import the wrong data ....

It gives Error: [Errno 13] Permission denied: '/tmp/dbgen.ldif' error when /tmp/dbgen.ldif already exists.
Maybe the tool should ask something like '/tmp/dbgen.ldif' already exists. Do you want to overwrite it? Or something like this...

Simple option here, why not write to the ldif dir of the related instance? If we plan to import it to that instance, then this would make the most sense because it then isolates between instances, and means the instance can actually read the ldif dir (a common issue with the import from say /root or /tmp is the dirsrv user of the instance can't read the directory the ldif is in ...)

Well not all of these LDIFs are designed to be imported. Groups, COS, Roles and modification ldifs are design to be used by ldapmodify, not ldif2db. And smaller user LDIF's could also be added using ldapmodify. Now I did think about using the server's LDIF directory since that information is available, but I also want all LDIF options to be consistent and forcing all LDIFs into the server's LDIF directory is not something I want to do, especially since this is all for testing and not for production, and most of the LDIF options are not meant for ldif2db.

Now in the interactive mode I added checks to see if the file can be written, if it can't it keeps prompting for a valid path/name. So permission issues and invalid paths are covered in the interactive mode.

Anyway let me work on all of this and I'll try and come up with something that works for all the cases. :-)

All changes applied @spichugi and @firstyear, please review...

Looks really good! You have my ack, at least. :)

It's a yay from me.

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

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.