IDview

[madhuriupadhye]

No description provided.

[madhuriupadhye]

@danlavu can you roughly overview this?
Please let me know for any changes

[danlavu]

I don't think this is how we should do it because this won't work for users. The command is different for users, groups and hosts, so we can rename the class to reflect the corresponding object. Maybe we should make a subclass or a method in the existing class, like

ipa.user("test user").add_idview(name="override_user")
ipa.host(["client"]).add_idview(["override_host"](

or a subclass

ipa.user("test user").add().override(name="override_user").add()

The thing I would like to see is that the parameters match the corresponding values in the parent class, so it's easier to write. The args for home directory in ideviews is 'homedir" but we are using "home" as an example.

Does this sound good?

[madhuriupadhye]

I don't think this is how we should do it because this won't work for users. The command is different for users, groups and hosts, so we can rename the class to reflect the corresponding object. Maybe we should make a subclass or a method in the existing class, like

ipa.user("test user").add_idview(name="override_user")
ipa.host(["client"]).add_idview(["override_host"](

or a subclass

ipa.user("test user").add().override(name="override_user").add()

The thing I would like to see is that the parameters match the corresponding values in the parent class, so it's easier to write. The args for home directory in ideviews is 'homedir" but we are using "home" as an example.

Does this sound good?

I don't think this is how we should do it because this won't work for users. The command is different for users, groups and hosts, so we can rename the class to reflect the corresponding object. Maybe we should make a subclass or a method in the existing class, like

ipa.user("test user").add_idview(name="override_user")
ipa.host(["client"]).add_idview(["override_host"](

or a subclass

ipa.user("test user").add().override(name="override_user").add()

The thing I would like to see is that the parameters match the corresponding values in the parent class, so it's easier to write. The args for home directory in ideviews is 'homedir" but we are using "home" as an example.

Does this sound good?

I am planning to create IDView complete different class, and overrirde will be subclass of user but let me create the document so it will be more clear.

[danlavu]

A few more directional changes, but this is looking a lot better, thank you.

[danlavu]

You should use the CLIBuilder to make it consistent with the rest of the code and you do not have to add conditionals.

https://pytest-mh.readthedocs.io/en/latest/articles/tips-and-tricks/building-command-line.html

[danlavu]

Would rename it to modify and change the execution to host.conn.run or host.conn.exec so you don't have to sync the name of the function.

[danlavu]

So, after some testing the same function name is overloading the IPAUser.add() function. I know this was my suggestion, and I'm mistaken. We can rename the function to __add() to avoid the overloading.

[danlavu]

Try this.

            if attrs is None:
                args = []
            
            attrs = self.cli.args(
                {
                    "desc": (self.cli.option.VALUE, description),
                    "login": (self.cli.option.VALUE, login),
                   "uid": (self.cli.option.VALUE, uid),
                    "gidnumber": (self.cli.option.VALUE, gid),
                    "gecos": (self.cli.option.VALUE, gecos),
                    "home_directory": (self.cli.option.VALUE, home_directory),
                    "shell": (self.cli.option.VALUE, shell),
                    "sshpubkey": (self.cli.option.VALUE, sshpubkey),
                    "certificate": (self.cli.option.VALUE, certificate),
                }
            )
         
            self.role.host.conn.exec(["ipa", "idoverrideuser-add", idview_name, self.name, *attrs])
            return self

[madhuriupadhye]

No, it did not work.

Command:
    ipa idoverrideuser-add newview1 user-4 desc login uid gidnumber gecos home_directory shell sshpubkey certificate
  CWD:
  Env:
  Output:
  Error output:
    ipa: ERROR: command 'idoverrideuser_add' takes at most 2 arguments

[danlavu]

    def __add(
        self,
        idview_name: str,
        *,
        description: str | None = None,
        login: str | None = None,
        uid: int | None = None,
        gid: int | None = None,
        gecos: str | None = None,
        home_directory: str | None = None,
        shell: str | None = None,
        sshpubkey: str | None = None,
        certificate: str | list[str] | None = None,
    ) -> IDOverride:

        attrs = {
            "desc": (self.cli.option.VALUE, description),
            "login": (self.cli.option.VALUE, login),
            "uid": (self.cli.option.VALUE, uid),
            "gidnumber": (self.cli.option.VALUE, gid),
            "gecos": (self.cli.option.VALUE, gecos),
            "homedirectory": (self.cli.option.VALUE, home_directory),
            "shell": (self.cli.option.VALUE, shell),
            "sshpubkey": (self.cli.option.VALUE, sshpubkey),
            "certificate": (self.cli.option.VALUE, certificate),
        }

        self.role.host.conn.exec(["ipa", "idoverrideuser-add", idview_name, self.name] + list(self.cli.args(attrs)))

        return self

Working now, the clibuilder adds -- to the command, the way you had it, it was doing ----uid. The _modify function, takes the dict and turns it into a list, so we had to do that too.

[danlavu]

I'd use just name instead of idview_name, since it's a different class.

[danlavu]

Which id view takes priority, is it the last one applied? I'd put that information in the :description:

Applies id view to host or hostgroup. 

:description: ID View is a key. Any overlapping views will replace the value. 
:param hosts: Host or hosts to apply the ID view to, defaults to None.