changing client options on a built instance

[sctt]

Hello,

I'm using PAMI for a big project, it works pretty well, but i'm experimenting a flexibility problem.

in substance, i need to change certain client options (like connection parameters) on run time, without losing others object data (like registered events).

currently, the only way to set options is trough a new instance, but this cause all previous data to be lost.

the client would be a lot more flexible with something like this :

[...]

    public function setOptions(array $options)
    {
        if (isset($options['host']))
            $this->host = $options['host'];
        if (isset($options['port']))
            $this->port = intval($options['port']);
        if (isset($options['username']))
            $this->user = $options['username'];
        if (isset($options['secret']))
            $this->pass = $options['secret'];
        if (isset($options['connect_timeout']))
            $this->cTimeout = $options['connect_timeout'];
        if (isset($options['read_timeout']))
            $this->rTimeout = $options['read_timeout'];
        if (isset($options['scheme']))
            $this->scheme = $options['scheme'];
    }

    public function __construct(array $options)
    {
        $this->scheme = 'tcp://';
        $this->setOptions($options);
        $this->logger = new NullLogger;
        $this->eventListeners = array();
        $this->eventFactory = new EventFactoryImpl();
        $this->incomingQueue = array();
        $this->lastActionId = false;
    }

[...]

let me know if you'd like to merge this improvement, i'm really going to need it on production.

thank you.

[marcelog]

Hello!

Could you explain the use case? It seems a bit hard to understand why you would like to change the connection parameters at runtime:

1. Once you're connected, if someone changes the username or password your connection is not affected until it closes.
2. If your connection drops and you have a stale user/password, a workaround would be to reload the connection information when creating (or re-establishing) a connection (btw, this should rarely happen, so it should not be that expensive).
3. If you create a connection and the host is changed, that connection drops immediately (and we can go back to point 2).

If you want to keep registered listeners, just create a builder function or method for the PAMI client, that will get the connection information and register all the listeners needed before returning the client. Also, if you have several asterisk boxes, create one PAMI client for each one of them.

Thoughts?

[sctt]

hi marcelog, sorry for the delay (i've got some health issue), and thank you for your interest.

i'll expose you a couple of personal scenarios where i actually need a function like setOptions.

Case I : change server dynamically in my environment users interact with a dynamic web application which communicate with the server trough websockets. they can change their pbx server options and the event listened manually, without reloading the page. without a setOptions function i should implement alternative solutions which add more complexity to my design. first of all, let's see what i can't do:

1. use a builder function, since events are chosen by users they can't be hardcoded.
2. use a client for each connection, since parameters are chosen by users i can't preload them. so, that's what i can actually do:
3. registering events in some other class. if users change listened events i have to memorize them, with their handlers and their predicates, outside ClientImpl and inject them to a new instance every time options change. i see that as a memory waste, especially if there are hundreds of events with different predicates, they will always been kept in memory twice. i could directly fetch events config from database everytime options change, but i keep seeing this as a performance waste: why should i reload all the events from disk if i am only changing the options?? this solution smells also of unneeded complexity. why should i define a redundant event registering class, rebuild completely the pami instance and inject all the events, when i could do something like this:

$client->close();
$client->setOptions($options);
$client->open();

Case II : a design problem on my server side, the system is well designed with high cohesion a very low coupling. i can't explain the whole architecture here of course, but i'll try to give you a simplified vision of the components with are related to this setOptions problem.

1. my system is a generic pbx manager, it doesn't use only asterisk, it uses also other technologies such as hylafax. i have a pbxManager class which offers methods to work with your pbx, regardless of the actual technology.
2. pbxManager doesn't know anything about the implementations, it uses a pbxInterface which has been extended in order to provide the specific coding.
3. a pbxConnector class stores all default connections and it is used by pbxManager, which will pass a pbxInterface in order to open the stream.
4. every pbxInterface is loaded from a config file, they are built as services and cached on disk for performance issues.
5. default connections are also loaded from a config file, but they are also loaded from database on a second stage.

i don't know if you can see the problem here already. my system FIRST load the client, and THEN the connections. that's how the design works. in this case, i don't need setOptions in order to change the parameters, but to define them on the first place, because when the client instance is built, the system doesn't know about connections yet. changing the design is out of question, because it is pretty S.O.L.I.D. and works pretty well with other implementations but PAMI. i could use some alternative solutions like the one expressed before, but they will smell here also of waste of performance and unneeded complexity.

i can see your point anyway: if someone change options without reloading the connection, bad things will happen. since i take that for granted, i automatically think that all the users that use PAMI has to be aware of this: changing the options must be followed by a connection reloading. but maybe i am wrong, so we can find a way to help users to understand that, maybe with something like this:

public function changeOptions(array $options, $reloadConnection = false)

in that way they will have a sort of hint, and in the case that reloading is actually needed in the same place where the options are changed, they will save some lines of code.

that's my thoughts, let me know what you think.

thank you again!

[marcelog]

Hello,

Sorry to hear about your health problem, I hope you get better!

Thank you for the detailed explanation, I see your points. I'll try to explain my point of view giving what you said.

It seems to me that you are using the PAMI client as a way to persist part of your application state (credentials, listeners, etc). This doesn't seem right at first, at least.

This means that yes, you should (in my personal opinion, of course) persist this information somewhere else and then create the client in your wrapper classes, this will let you further decouple your code and manage the connections in better ways.

• You could change implementations on the fly if desired (let's say you don't want to use PAMI anymore).
• You can have this information available somewhere else if your code (or PAMI's) crashes (i.e: you can recover operations smoothly once your process restarts by reloading the connections and listeners, otherwise without persisting this anywhere, your users will just lose what they have setup).
• It doesn't seem right that you let your users execute code directly in their listeners (i.e: in your server), so it also seems like you should abstract the listeners code and only let them do some specific, well known actions that you can control, so saving this part of your application state translates to saving the metadata of what the user wants to do (you can then inject the needed code in the listeners so it complies with this metadata, but you don't let them run their code directly in the server, this seems like a security issue).
• It's not necessary to store this in memory, you can use a NoSQL database, or even disk (as I said, once everything is setup, how many times the credentials or listeners will change?).

So overall, this is what I'd suggest:

• To have a supervisor for the connections:
  • This is a daemon running in your server as a background process. This might be all of your PBX wrapper classes.
  • Could be one per client and one supervisor on top of all of them that restarts any of them if a crash is detected.
  • Could also be just one for all the clients (you could use something like daemontools to keep this one restarting if it crashes).
  • A supervisor can have any number of dynamically allocated connections, you can start and stop them easily, on demand, they don't need to be started when the supervisor starts (although they could!) and they can be started/stopped by your GUI commands.
• Your GUI will communicate with a few web controllers, either by REST or websockets, and should get events by either longpolling, websockets, or SSE.
• Your GUI or controllers can trigger actions in your system (like for example credentials changed, new listeners needed, etc) by sending a command through the websocket or via REST.
• Your controllers should dispatch that command through a message bus, could be rabbitmq, a redis queue, 0mq, etc.
• Your supervisor(s) will listen for those events and react (for example restarting a connection -and also reloading any listeners-, or by adding/removing listeners, etc). This can translate into creating or destroying any number of PAMI clients without needing to change their information (and this is actually the intended way to use PAMI).

So we do have different views I guess, and I'm not yet convinced that this change will benefit all PAMI users (I do think that there is some more decoupling yet to be done), but the good news is that you can fork the code if this is really the way you want to work with it.

Best!

[sctt]

hi marcelog,

you are really kind, thank you, your explanation is pretty long and reasoned considering that this problem doesn't really concern you. thanks.

i see your points and i can agree in a certain sense. a setOption method may be a fast way to solve my requirements without breaking the design, but actually relying on pami to memorize my event configuration doesn't seem the best option.

coding a daemon and/or new wrapper classes will introduce bigger efforts to me, but i think the design will result to be stronger.

i'll try what you suggested before forking.

thank you very much for your interest.

[marcelog]

Great, thank you @sctt! I will close this one now then, but please get back with the results once you settled for one solution or the other, it's an interesting topic and discussion :)

All the best!