Skip to content
This repository has been archived by the owner on Jan 5, 2022. It is now read-only.

Using things loaded from the ini plugin in setup() #1

Closed
vitch opened this issue Oct 5, 2011 · 6 comments
Closed

Using things loaded from the ini plugin in setup() #1

vitch opened this issue Oct 5, 2011 · 6 comments

Comments

@vitch
Copy link
Contributor

vitch commented Oct 5, 2011

Hi,

First up thanks for this code. Just playing around with it and it seems like a nice clear bit of code which does exactly what I need!

I've run into a problem because I am storing database config in a config file loaded by Core_Plugins_Ini. I want to create the database connection in my daemon's setup() function. However, due to the order that things happen the setup() in Core_Plugins_Ini hasn't been executed by the time my daemon's setup() function is called.

How would you normally suggest avoiding this problem? One solution would be to init the plugins before the main daemon (around https://github.com/shaneharter/PHP-Daemon/blob/master/Core/Daemon.php#L269 ) but I'm not sure if this would have unwanted side effects?

The other solution (which I'm currently using as a workaround) is to override run() to look like this:

public function run()
{
    $this->init_db();
    parent::run();
}

This seems pretty hacky though. I guess the other solution is to check in execute() whether the connection has been created and create it if now.

How do you recommend dealing with this situation?

Thanks!

@shaneharter
Copy link
Owner

Vitch,

Thanks for the note. I'm glad you found it useful!

Let me think about this for a bit, I'll get back to you. But I will share my own personal preferred setup, which is to keep DB connection info in an include of some kind. This is also a natural fit if you're using a DB library that's shared with the web-facing part of your codebase.

But, you present an interesting idea that while I don't do it that way, I can see how it could be a very common use case. I'm sure there's an elegant solution to be found.

Shane

@vitch
Copy link
Contributor Author

vitch commented Oct 6, 2011

Hey Shane,

Thanks for the reply :)

In my case there is a bunch of connections that I want to create in the setup() function (I have some different daemons which each connect to one or more of a MySQL database, a RabbitMQ queue, the Twitter firehose and Amazon S3). It seems nice to me for the required details for these connections to be stored in a config file. It's also nice that I can load a different ini file based on an environment variable (with the settings for e.g. dev vs production) and that everything related to configuration is stored in one place.

I could have this stuff in a different include file but then I'm not sure what I'd end up using the Core_Plugins_Ini plugin for.

Anyway - thanks again for the library and having a think about my question,

Kelvin :)

@vitch
Copy link
Contributor Author

vitch commented Oct 6, 2011

Hey,

This is actually effecting me in quite a few ways now... I'm thinking that for my specific code I'm going to just swap around the order that setup() is called on the daemon and plugins (by moving line 270 below line 274 in Core_Daemon).

Can you see a downside to this? Do you have situations where you have plugin code whose setup relies on the deamon's setup already running?

Cheers,

Kelvin :)

@shaneharter
Copy link
Owner

Alright, I remember this now.

The issue then is that if you load any plugins in your setup() method, those
plugins will not have their setup() called.

I suppose it could be in the docs that you have to load any plugins you want
to use in your constructor, but generally I prefer to keep constructors
minimalistic. I've seen some strange php error messages when uncaught
exceptions are thrown from a constructor.

Though it's definitely a best practice on a daemon like this to use a global
exception handler, so that could be the answer there.

On the other hand, it would be better to setup the lock plugin before your
daemon setup gets called -- it avoids a needless call to setup() in the case
where a lock is present and the instance is killed 5 lines later in the
init() method.

I think for a personal fork, if you wanted to follow the guideline of
loading plugins in your constructor only, you could make that change on your
fork without any other side effects.

On Thu, Oct 6, 2011 at 11:26 AM, Kelvin Luck <
[email protected]>wrote:

Hey,

This is actually effecting me in quite a few ways now... I'm thinking that
for my specific code I'm going to just swap around the order that setup()
is called on the daemon and plugins (by moving line 270
below line 274
in Core_Daemon).

Can you see a downside to this? Do you have situations where you have
plugin code whose setup relies on the deamon's setup already running?

Cheers,

Kelvin :)

Reply to this email directly or view it on GitHub:
#1 (comment)

@vitch
Copy link
Contributor Author

vitch commented Oct 6, 2011

Ahh - that makes sense... The ini file was loaded in the constructor in the example so I presumed that was just how to do it...

The argument about the lock plugin also makes sense - I don't want the overhead of creating my DB connections etc when it will be all wasted.

However, by the time I read this I'd already done the changes on a fork. And refactored some of my other code to work with the changes.

Actually, looking at the code I think that the lock check is always performed after both the plugins and main daemon have had their setup() called. So I guess it doesn't matter from that point of view... Maybe the order should be Plugin setups > lock check > daemon setup. Although that still leaves you with the "init plugins in the constructor" limitation... I guess it depends on what other plugins you have... I only have the lock and config ones so far...

@vitch
Copy link
Contributor Author

vitch commented Oct 7, 2011

I put together an example of a possible approach in pull request #3 - have a look through the commits and their comments and let me know if you think it's a good solution. It seems to be working for me but as I say I'm currently only using the ini and lock plugins so I might be missing something more...

@vitch vitch closed this as completed Oct 10, 2011
ekzobrain added a commit to ekzobrain/PHP-Daemon that referenced this issue Jun 15, 2013
2. Improved memory allocation in SysV
3. Fixed notice on exit, reason was cyclic reference when destroying objects:
PHP Notice: Undefined property: Core_Worker_Via_SysV::$mediator in /opt/PHP-Daemon/Core/Worker/Via/SysV.php on line 95 pid 16905
#0  Core_Worker_Via_SysV->setup_ipc() called at [/opt/PHP-Daemon/Core/Worker/Via/SysV.php:521]
shaneharter#1  Core_Worker_Via_SysV->release() called at [/opt/PHP-Daemon/Core/Worker/Mediator.php:258]
shaneharter#2  Core_Worker_Mediator->__destruct() called at [/opt/PHP-Daemon/Core/Worker/Via/SysV.php:53]
shaneharter#3  Core_Worker_Via_SysV->__destruct()
@vitch vitch mentioned this issue Jul 9, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants