Added new command version. Query the server for client's read-only status.

[EricPei20]

No description provided.

[CSSFrancis]

I think that it might be better to have the read_only as a getter/setter.

    @property
    def read_only(self):
        command = self._addSingleCommand(self.GET_CLIENT_READ_ONLY, None)
        response = self._sendCommand(command)
        if response != False:
            read_only = self.__getParameters(response.acknowledge[0])[0]
        else:
            read_only=None
        return read_only

    @read_only.setter
    def read_only(self, read_only):
        command = self._addSingleCommand(self.SET_CLIENT_READ_ONLY_DEPRECATED, None, [read_only])
        response = self._sendCommand(command)

So then you could have client.read_only=False etc...

Right now the problem is that you can do client.read_only=True and the value will change but nothing will actually change... There are lots of things like if the set fails for some reason the value will not actually update.

[CSSFrancis]

Also if you want the pre-commit test to pass that just requires

pip install pre-commit
pre-commit install

That installs the pre-commit hook to the repo so it will run a code formatting check/ style update every time that code is committed. Just something that helps to keep things easier to maintain.

[CoolFanyu]

I guess we don't need to make the read-only client to have option set/get the read-only? I don't think this is a real use case that user need and our server does not support this currently.  And if user need to have more read-write client, they need to set the attribute MaxReadWriteClients in server.xml so Server allow to have another read-write client connect. And server created a different port for Read-only client, so port will decide the client is read-only or read-write which is much easier to implement in our code.

I think that it might be better to have the read_only as a getter/setter.

    @property
    def read_only(self):
        command = self._addSingleCommand(self.GET_CLIENT_READ_ONLY, None)
        response = self._sendCommand(command)
        if response != False:
            read_only = self.__getParameters(response.acknowledge[0])[0]
        else:
            read_only=None
        return read_only

    @read_only.setter
    def read_only(self, read_only):
        command = self._addSingleCommand(self.SET_CLIENT_READ_ONLY_DEPRECATED, None, [read_only])
        response = self._sendCommand(command)

So then you could have client.read_only=False etc...

Right now the problem is that you can do client.read_only=True and the value will change but nothing will actually change... There are lots of things like if the set fails for some reason the value will not actually update.

[CSSFrancis]

Did we decide that the port sets if the client is Read only?

This is a change to our API and might break our users code so we need to be careful about and need to be much better about documenting it.

1. Do we need to do this?
2. If we need to do this then we should use proper deprecation:

deapi/deapi/wrappers.py

Line 35 in b4c75af

class deprecated_argument:

[CSSFrancis]

Suggested change

	@deprecated_argument(name="read_only", since="0.5.3", removal="0.5.3", alternative="port =13241")
	def connect(self, host: str = "127.0.0.1", port: int = 13240, read_only=False):

[EricPei20]

We have decided to have the ports determine if the client is read-only, so changes have already been made in the server to support this (as of 2.8.3.12436). Both the read-write port (default 13240) and read-only port (default 13241) are also configurable now in server.xml.

[CSSFrancis]

I'll edit this to make the deprecation and then merge then. It shouldn't be that widely used...

[EricPei20]

I thought deprecated_argument is meant for renaming arguments that have already been removed. In this case, there is no renamed "alternative" for read_only and we're also not removing the read_only argument yet, since we need the updated api to be backwards compatible with older server versions.

Maybe I can just add something like:

Philip agrees with this change.

[CSSFrancis]

I guess I don't love that this breaks our API in quite a few ways.

For example what happens if someone has existing code that uses 13241 as a read/write server. Or if someone has code where read_only=True and it breaks on a minor release. I guess I realize why the change is necessary but we are way too quick to break our API and it's one of my pet peeves. Even changing the names of properties in our API should be well documented and come with a deprecation cycle. Lots of people build things on top of our API and we can't just change things without a heads up.