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
On Tue, May 5, 2020 at 10:21 AM Travis <tel...@tripadvisor.com> wrote:
> 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
>
> On Tuesday, May 5, 2020 at 12:07:30 PM UTC-4, David Trowbridge wrote:
>>
>> 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
>>
>> On Thu, Apr 30, 2020 at 11:40 AM Travis <tel...@tripadvisor.com> wrote:
>>
>>> 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
>>>
>>> On Friday, April 17, 2020 at 5:59:24 AM UTC-4, Petr Grenar wrote:
>>>>
>>>> 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.
>>>>
>>>> [image: log.png]
>>>>
>>>> [image: extension.png]
>>>>
>>>>
>>>> Thank you in advance.
>>>>
>>>> Petr Grenar
>>>>
>>> --
>>> 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 revie...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/reviewboard/115aa918-95e3-421c-923e-83684b7386cd%40googlegroups.com
>>> <https://groups.google.com/d/msgid/reviewboard/115aa918-95e3-421c-923e-83684b7386cd%40googlegroups.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>> --
> 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+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/reviewboard/48902c75-b28e-4c36-9aaa-fd0c01c01b28%40googlegroups.com
> <https://groups.google.com/d/msgid/reviewboard/48902c75-b28e-4c36-9aaa-fd0c01c01b28%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
--
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/reviewboard/CAFS3VNV95ZsEs%2BqOiiwt16HuagiiiwXnLpGwqKcjii84fx2APA%40mail.gmail.com.
