danog / MadelineProto

Async PHP client API for the telegram MTProto protocol
https://docs.madelineproto.xyz
GNU Affero General Public License v3.0
2.87k stars 662 forks source link

Update Settings does not accept array as pointed out in the docs #939

Closed bytes-commerce closed 2 years ago

bytes-commerce commented 3 years ago

Hello! Nice library. I am using it to play a little bit around.

I figured that I cannot update the settings once the session has created in memory/files, so I tried to follow this documentation: https://docs.madelineproto.xyz/docs/SETTINGS.html#settingsapp_infoapi_id and it points out that you can run $mp->updateSettings([]); but when debugging the class it expects a SettingsAbstract.

Instead, to update the settings to use now MySQL instead of memory, you have to use this call: $c->setDb((new Mysql)->setDatabase('madeline')->setUsername('madeline')->setPassword('madeline')->setUri('database'));

Where Mysql is an Instance of use danog\MadelineProto\Settings\Database\Mysql;. Would be great if you could update the docs!

Thanks!

danog commented 3 years ago

Yeah, still gotta do that in preparation for the final v6 release :) In the meantime you fan check out https://docs.madelineproto.xyz/PHP for the PHP api docs!

bytes-commerce commented 3 years ago

Thanks for your work! I am currently checking whats going on, when I change the settings and re-init the API, it seems to use cached configs, which of course is not ideal when I change the settings. I pass this: $this->mp = new API('index.madeline', $this->config->getDefaultConfig()); And I pass a ['logger' => ['logger_level' => 0]] to the constructor, but it does not update accordignly. I can run the updateSettings function tho, but I do not want to do it every time, because I already pass the updated configs to the constructor.

This seems to be a bit buggy at the moment or I am making a mistake somewhere.

bytes-commerce commented 3 years ago

The reason for that is here: src/danog/MadelineProto/Settings/Logger.php::203.

This call specifies the minimum logging activity, but maybe this is rather a leftover developer setting, can that be? Does the force of that serve any deeper purpose (like allowing a user to login via CLI?): $this->level = \max($level, MadelineProtoLogger::NOTICE);

danog commented 3 years ago

Indeed, passing settings to the constructor will 100% apply them only if you are using the event handler: otherwise, they will not be applied properly.

The reason behind this is the need to reduce startup latency when using IPC, avoiding the additional overhead of changing the settings on every startup if they haven't changed at all (just like you don't want to run updateSettings every time on startup). Logically, if you need to change your settings, you already need to manually edit a file, then why not simply create another file specifically to call updateSettings once, and be done with it.

About logging, it cannot be disabled to allow better debugging of userbots created with it, but a logfile size limitation can be set, instead (for now 25MB, in future will be lowered).

bytes-commerce commented 3 years ago

Okay, thanks for the explanation. Since it appears that the config is cached somewhere might it not make sense to run a quick array_diff on the "new" settings and re-apply only those changed? Otherwise, I cannot feel the impact on performance at the moment if I simply run updateSettings with the respective setting being changed. Let me know your thoughts on this, thank you a lot for the effort you put into this project! :)

phamxuanloc commented 3 years ago

Thank for the explaination. But now I still don't know how to update the settings to use now MySQL instead of memory. Please guide me. Thank you

bytes-commerce commented 3 years ago

@phamxuanloc you can do the following to set MySQL for every request:

    $this->mp = new API('index.madeline', ['your settings']);
    $this->mp->updateSettings($this->mp->getSettings()->setDb(
        (new Mysql)->setDatabase('<yourDbName>')
            ->setUsername('<yourUser>')
            ->setPassword('<yourPassword>')
            ->setUri('<yourHost>')
    ));

Dont forget to adapt it to your needs. Remove the madeline files to re-apply the settings once, then you can dynamically update them via updateSettings.

phamxuanloc commented 3 years ago

@phamxuanloc you can do the following to set MySQL for every request:

    $this->mp = new API('index.madeline', ['your settings']);
    $this->mp->updateSettings($this->mp->getSettings()->setDb(
        (new Mysql)->setDatabase('<yourDbName>')
            ->setUsername('<yourUser>')
            ->setPassword('<yourPassword>')
            ->setUri('<yourHost>')
    ));

Dont forget to adapt it to your needs. Remove the madeline files to re-apply the settings once, then you can dynamically update them via updateSettings.

Hi, Thank for your support. I setting the mysql direct to the constructor first and then use updateSetting. The constructor update config db is mysql but it cannot update mysql dbname and user. So I got error. Now I remove db config on constructor and use updateSetting. It's ok now. Thank for your help.

phamxuanloc commented 3 years ago

Dont forget to adapt it to your needs. Remove the madeline files to re-apply the settings once, then you can dynamically update them via updateSettings.

The _madeline_file alway auto generate even if I use either Mysql database or memory? right? Do you mean some .lock, .ipc, .callback .... file ?

danog commented 3 years ago

No, you should never remove Madeline files, this will cause a crash. Just updateSettings.

bytes-commerce commented 3 years ago

Ah, interesting! Thats probably the reason why I cannot move the whole project from one instance to another (my local machine -> my AWS server/my raspberry pi). :+1: Thanks for the insight.

danog commented 2 years ago

The settings docs are now updated: https://docs.MadelineProto.xyz/docs/SETTINGS.html