argsn - typo?

Nice catch, fixed!

There is no need for this kind of changes. Module 'argparse' takes care about it and takes the variable name from stripping '--' arg. '--verbose' in this case.
https://docs.python.org/3.6/library/argparse.html#dest

The way I read that doc was that you had to set dest if you used "-v" AND "--verbose" Only if it's just "--verbose" is it automatically included. Have I misread it?

Hmm, I refer to this part of the doc:
"For optional argument actions, the value of dest is normally inferred from the option strings. ArgumentParser generates the value of dest by taking the first long option string and stripping away the initial -- string. If no long option strings were supplied, dest will be derived from the first short option string by stripping the initial - character. "

Also, I've tested your patch without "dest='verbose'" change and it works. :)
Still, it is not a bit issue. I am okay if it will stay like this.

I can change it, no problem since you verified it still works....

Changes made, please review...

ldap_url - looks like a typo

The 'list' method works

[root@qeos-55 ds]# dsidm -D "cn=Directory manager" --json -b "dc=example,dc=com" ldap://localhost:389 user list
Enter password for cn=Directory manager on ldap://localhost:389 :
{"type": "list", "items": ["user1", "user2"]}
Command successful.

But I have an error with 'get' method:

[root@qeos-55 ds]# dsidm -D "cn=Directory manager" --json -b "dc=example,dc=com" ldap://localhost:389 user get
Enter password for cn=Directory manager on ldap://localhost:389 :
Enter uid to retrieve : user1
Error: 'nsUserAccounts' object has no attribute 'get_all_attrs_json'

Do I need to apply some another patch? Or it will be just added later? I see you said it is an initial patch.

Weird, so the _generic_get() is being called from lib389/cli_base/init.py in your case, but I would expect _generic_get() should get called from lib389/cli_idm/init.py when using dsidm (which uses get_json() not get_all_attrs_json()).

So is something broke with the model? Needs more investigation...

Well I fixed the model but I haven't verified it yet. I will do so tomorrow...

A couple of issues here...
First, you use _get_arg function and it fails on Python 3 because it uses raw_input() function which is not present. We should use input() built-in function instead.

Another thing, 'create' function fails

dsidm -v -D "cn=Directory manager" -b "dc=example,dc=com" ldap://localhost:389 user create
Traceback (most recent call last):
  File "/usr/sbin/dsidm", line 109, in <module>
    args.func(inst, dsrc_inst['basedn'], log, args)
  File "/usr/local/lib/python3.6/site-packages/lib389/cli_idm/user.py", line 42, in create
    kwargs = _get_attributes(args, SINGULAR._must_attributes)
  File "/usr/local/lib/python3.6/site-packages/lib389/cli_idm/__init__.py", line 41, in _get_attributes
    for attr in attrs:
TypeError: object of type 'Namespace' has no len()
args = Namespace(basedn='dc=example,dc=com', binddn='cn=Directory manager', cn=None, displayName=None, func=<function create at 0x7f3d1741e598>, gidNumber=None, homeDirectory=None, instance='ldap://localhost:389', json=False, starttls=False, uid=None, uidNumber=None, verbose=True)

But I think it is not the purpose of the patch so we can skip it for later. It is easy to fix though. :)

I have an error here:
AttributeError: 'nsUserAccounts' object has no attribute 'get_json'

It fails with 'NameError: name 'json' is not defined'. I think we miss 'import json' in this file.

Sorry I haven't verified my last patch yet. I wanted to get to it yesterday but that didn't happen, but I'll be working on this later today. Thanks for the testing though!

Sorry I haven't verified my last patch yet. I wanted to get to it yesterday but that didn't happen, but I'll be working on this later today. Thanks for the testing though!

Oh, sorry... I've seen the rebase...

Okay, thank you!

//POC REMOVE!!! :)

You probably want to get rid of this line.

Please don't do this. The point is to have the LOWEST exception propogate because it has the LDAP info about WHY this failed. If you re-raise, we lose that knowledge, and then it becomes creally hard to trace issues. Please NEVER try-reraise in python.

_json is a magic type that is suffixed to any function of DSLdapObject, so for example, get() can be used as get_json() BUT it relies on the function having a specific output type.

No, because you may corrupt the data here. str in python 3 IS unicode, so you need to either return it as bytes, or just ALLOW the exception to be raised.

IE: Just let the exception be raised because we can't help you after this happens.

A better spot to put this is probably in the dsrc handling, because "ldapurl" is a derivation of "instance name". So remove this, and move it to the dsrc.py so that if instance name == ldap, then you use the ldapi:// url of the instance instead. You could even do something like local_ and have that generate it. This hardcoded here is not right sorry.

The CN is required though,

You may want a "json success" type that you could return here instead.

See belowe, this is the wrong location for this code,

You can probably remove this change if you change the belowe ldapi settings to be part of dsrc.

Hey there, lots of comments for you. Hope it helps :)

I want to keep this example in the code until we actually have functions that use it properly. Then I will remove :-p

already did, I'm working on this right now so you are commenting a bit early. Wait a few hours I'll have a more revised patch to look at.

cockpit will just use the result code from the cli tool to know if its a success or failure. I thought about doing a "result" JSON object, but its not needed, and I don't want to add the extra complexity at this stage. That being said I could jsonify it for the _generic results, but the UI won't use it.

Yeah I wasn't sure if I should of been doing that. I'll remove it...

Okay, well your can't "require" an argument what has nargs='?'. I'll look into this see if nargs should be changed so we can bring the requirement back.

I've fixed this already... the issue is that in this context mc is nsUserAccounts - so get_json() doesn't work as its a list, not an entry. Like I said, I have a fix for this and it will be in the next rebase.

So this happens when we try and get the nsState attribute from a replica. I know it can not read it, but the value should NOT cause an error. This was a really hack fix to get around it, but I'm not sure how to address it correctly.

I'll look into this next...

@firstyear - about to add dsrc changes next...

dscreate and 'dsidm user get' work. But 'dsidm user list' fail with 'NameError: name 'json' is not defined' failure. We dont have an import here... Sorry if you are already up to this

Fixed!

Also started adding JSON schema support, and added missing dsconf options for getting objectclasses and matching rules.

We need to rework how we use query_attribute/query_objectclass. We need a way to convert the python-ldap AttributeType class to json object, same for objectclass and matching rule ldap classes. I'll be working on that next....

Solved the AttributeType/Objectclass class to json issue,. Still need to properly implement the query functions for JSON...

Ready for review...

Tested... Looks good to me! Ack.

I'll merge this for now, and if can address any further issues in a new ticket/PR.

I'm still not happy about how I handled "nsState" binary value issue. I just treat it as a string on exception, but I'm not sure what is the best way to process it. The value is valid, it's not an error to be "handled". For the UI/CLI it's not actually an attribute we care about, but the json processing should not error out on it either.

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

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.