Extension is running multiple times

[Petr Grenar]

Hello,

I have a running extension that is finally doing what I want :) But there is one thing that I don't understand. The extension is working with SignalHook review_request_closed and a SampleApprovalHook. So when someone put ship it and submitted on review request the extension is called. This is working. But when I go to the apache log I can see that the extension was called multiple times with the same result. Is this behavior normal or am I doing something wrong?

The extension is calling other stuff but the log was the same even before I was calling the other things.

You can see the log output and extension on the attached images.

Thank you in advance.

Petr Grenar

[Travis]

I am running into the same situation as Petr. I have noticed the same behavior plus some other concerns (that I believe Petr's logs also show).

I put logging into my extension initialize, shutdown, and is_approved. When I enable my plugin and visit a RB page:

• My extension's initialize() and shutdown() are logged numerous times.
• This is contrary to the documentation which says that initialize should only fire when an extension is enabled and shutdown should only fire when disabled and/or the server is shut down.
• My ReviewBoardApprovalHook is_approved is logged 1-3 times.
• Expect that this should only fire 1x?

The problem gets worse if I disable my extension. Once disabled, I continue to see logs related to my extension's shutdown and is_approved until I restart apache. If I toggle my extension on and off, the number of is_approved calls continues to grow. 

It seems as though these hooks are not getting properly removed when the extension is disabled (despite what the documentation says). Possibly made worse because the extension is being initialized/shutdown multiple times per page render?

I did make one change to explicitly disable the ReviewRequestApprovalHook that I added in my extension's shutdown. This seemed to reduce the is_approved checks to only 1 per page render (and none once the extension is disabled), but I still see the extension being enabled/disabled multiple times and I suspect that is the true cause of this issue?

Any help/thoughts are greatly appreciated.

from reviewboard.extensions.base import Extension
from reviewboard.extensions.hooks import ReviewRequestApprovalHook
import logging

class ApprovalRulesExtension(Extension):

    def initialize(self):

        self.approval_hook = TestApprovalHook(self)
        logging.debug("Enabling ApprovalRulesExtension")

    def shutdown(self):

        self.approval_hook.disable_hook()
        logging.debug("Disabling ApprovalRulesExtension")

class TestApprovalHook(ReviewRequestApprovalHook):
    def is_approved(self, review_request, prev_approved, prev_failure):

        logging.debug('checking is_approved')
        return True

Thanks,

Travis

[David Trowbridge]

There are slightly different issues here.

For Petr's code: you're instantiating a new SampleApprovalHook every time a signal is fired. You'll want to just do it once in your extension's initialize() method.

For Travis, it sounds like you're running your extension code in a production setting with Apache. Apache has many processes and threads serving pages, and initialize/shutdown will be called every time a new thread spins up or shuts down, and all go into the same log file. As far as needing to remove the hook yourself, it's because you're overriding shutdown() without calling the superclass version. If you add super(ApprovalRulesExtension, self).shutdown() it will disconnect all hooks when shutting down.

David

--
Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "Review Board Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/115aa918-95e3-421c-923e-83684b7386cd%40googlegroups.com.

[Travis]

Thank you, David!

I'm kicking myself over the shutdown override. That makes perfect sense.

You are correct in that I am running this in a production apache environment. I had planned on doing some of the heavier lifting (service calls) in the initialize method assuming the results would be around for a while, but if this may be enabled/disabled multiple times per page render, it sounds like I am better off only executing these calls in the actual actions I am hooking into?

Thanks,

Travis

To unsubscribe from this group and stop receiving emails from it, send an email to revie...@googlegroups.com.

[David Trowbridge]

Travis,

In theory, Apache threads should be relatively long-lasting. But if you've just restarted the server, it will spin up a bunch of new threads and processes all at once. I wouldn't expect a bunch of shutdowns, but it depends on how your Apache workers are configured. I'd suggest that initialize is probably not a great place to do long-running tasks, because that can impact response times for all requests if a new thread is starting up, and you probably don't want to affect the performance of other parts of the application.

David

To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/48902c75-b28e-4c36-9aaa-fd0c01c01b28%40googlegroups.com.

[Travis]

Sounds good. Thank you for your help!

To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/48902c75-b28e-4c36-9aaa-fd0c01c01b28%40googlegroups.com.