Automatic reload of environments after trac.ini updates

Hi Trac devs,

I have been putting some thought into how trac.ini works -
and have
essentially two tickets that are problematic (and important)
to me,
and most likely many others would be interested in changed
behavior:
http://trac.edge
wall.org/ticket/5135
http://trac.edge
wall.org/ticket/3833

Seeing that the upcoming Workflow is also heavily depending
on
trac.ini for configuration, I looked into that last night
(thanks
Eli). Workflow loads config once on initialisation, and
needs a
restart to reload if you are using a typical
Apache/mod_python setup.
As Christian also commented on one of the mentioned tickets,
some
things like log configuration also requires reload -
essentially a
server restart.

Some things require restart, some things don't. To me that
is
unpractical, and does not feel like good behavior from an
application
that if run as old-style CGI have no problem reloading on
demand
(essentially for every request). It just feels like the 'way
things
are'.

I operate a commercial service based on Trac (http://
www.coderesort.com), and with 40+ and growing external
customer
projects it is not just a matter of restarting the server
whenever - I
want to provide flexible options for my customers, and not
have them
contact me whenever a restart is needed.

Finally, a simple solution appeared to me that solves these
issues,
and the patch is attached to ticket #3833 (patch for
0.10-stable
revision 5342):
http://trac.edgewall.org/attachment/ticket/3
833/env_reload_5342.diff

It just modifies trac.config.parse_if_needed() slightly to
add an
optional 'check_only' parameter that instead of actually
reparsing
just returns True/False if a new parse is needed. Then in
trac.web.main._open_environment() I just changed the test
for env in
cache to also check if the env is outdated - and reload a
fresh env if
so. It is a much better solution than trying to compensate
for
shortcomings in config parsing (#5135 and see also http://trac.edge
wall.org/ticket/3037).
Possibly there might also be other tickets and circumstances
that are
positively affected by this change.

As far as I can tell it works like a charm, and without
side-effects
of any kind - it adds arbitrary reload of eny env on any
running 'in-
memory-server', without interruption to any other projects.
Much, much
better than a server restart as far as I can tell. If you
enable a
component in WebAdmin 'Plugins' page it will actually
'enable'
immediately, 'disable' and it disappears.

Please, please can I ask you to include this patch in Trac -
also for
0.10-stable as that is my current setup (I certainly
consider it a
'bugfix'... ?

I welcome input and comments!


:::simon


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Trac Development" group.
To post to this group, send email to trac-devgooglegroups.com
To unsubscribe from this group, send email to
trac-dev-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

osimons wrote:
> ...
> Finally, a simple solution appeared to me that solves
these issues,
> and the patch is attached to ticket #3833 (patch for
0.10-stable
> revision 5342):
> http://trac.edgewall.org/attachment/ticket/3
833/env_reload_5342.diff
>
> It just modifies trac.config.parse_if_needed() slightly
to add an
> optional 'check_only' parameter that instead of
actually reparsing
> just returns True/False if a new parse is needed. Then
in
> trac.web.main._open_environment() I just changed the
test for env in
> cache to also check if the env is outdated - and reload
a fresh env if
> so...
>   

The approach looks good to me.

> As far as I can tell it works like a charm, and without
side-effects
> of any kind - it adds arbitrary reload of eny env on
any running 'in-
> memory-server', without interruption to any other
projects. Much, much
> better than a server restart as far as I can tell. If
you enable a
> component in WebAdmin 'Plugins' page it will actually
'enable'
> immediately, 'disable' and it disappears.
>
> Please, please can I ask you to include this patch in
Trac - also for
> 0.10-stable as that is my current setup (I certainly
consider it a
> 'bugfix'... ?
>   

Well, this I'm less sure about... this could have unwanted
side-effects, 
particularly w.r.t. to the cached resources (db pool,
repository pool). 
This is a new situation _ recreating potentially many times
the 
environment "live" as opposed to creating it once
and keeping it for the 
whole duration of the process _ and we shouldn't treat that
change 
lightly. So I propose to make the change on trunk first, and
only if 
that change proves to be unproblematic after a while, we'll
backport it.

-- Christian

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Trac Development" group.
To post to this group, send email to trac-devgooglegroups.com
To unsubscribe from this group, send email to
trac-dev-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

On May 7, 8:17 am, Christian Boos <c...neuf.fr> wrote:
>
> > As far as I can tell it works like a charm, and
without side-effects
> > of any kind - it adds arbitrary reload of eny env
on any running 'in-
> > memory-server', without interruption to any other
projects. Much, much
> > better than a server restart as far as I can tell.
If you enable a
> > component in WebAdmin 'Plugins' page it will
actually 'enable'
> > immediately, 'disable' and it disappears.
>
> > Please, please can I ask you to include this patch
in Trac - also for
> > 0.10-stable as that is my current setup (I
certainly consider it a
> > 'bugfix'... ?
>
> Well, this I'm less sure about... this could have
unwanted side-effects,
> particularly w.r.t. to the cached resources (db pool,
repository pool).
> This is a new situation _ recreating potentially many
times the
> environment "live" as opposed to creating it
once and keeping it for the
> whole duration of the process _ and we shouldn't treat
that change
> lightly. So I propose to make the change on trunk
first, and only if
> that change proves to be unproblematic after a while,
we'll backport it.
>
> -- Christian

Since we have the old env available before fetching a new
one, would
we add safety to this approach by calling env.shutdown()
before
leaving it to be garbage collected?

:::simon


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Trac Development" group.
To post to this group, send email to trac-devgooglegroups.com
To unsubscribe from this group, send email to
trac-dev-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

osimons wrote:
>
> On May 7, 8:17 am, Christian Boos <c...neuf.fr> wrote:
>   
>>> As far as I can tell it works like a charm, and
without side-effects
>>> of any kind - it adds arbitrary reload of eny
env on any running 'in-
>>> memory-server', without interruption to any
other projects. Much, much
>>> better than a server restart as far as I can
tell. If you enable a
>>> component in WebAdmin 'Plugins' page it will
actually 'enable'
>>> immediately, 'disable' and it disappears.
>>>       
>>> Please, please can I ask you to include this
patch in Trac - also for
>>> 0.10-stable as that is my current setup (I
certainly consider it a
>>> 'bugfix'... ?
>>>       
>> Well, this I'm less sure about... this could have
unwanted side-effects,
>> particularly w.r.t. to the cached resources (db
pool, repository pool).
>> This is a new situation _ recreating potentially
many times the
>> environment "live" as opposed to creating
it once and keeping it for the
>> whole duration of the process _ and we shouldn't
treat that change
>> lightly. So I propose to make the change on trunk
first, and only if
>> that change proves to be unproblematic after a
while, we'll backport it.
>>
>> -- Christian
>>     
>
> Since we have the old env available before fetching a
new one, would
> we add safety to this approach by calling
env.shutdown() before
> leaving it to be garbage collected?

Yes, and there is for sure some interactions with #5010 (see
in 
particular its comment:15).

-- Christian

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Trac Development" group.
To post to this group, send email to trac-devgooglegroups.com
To unsubscribe from this group, send email to
trac-dev-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---