Dave wrote:
On 6/13/07, Allen Gilliland <[EMAIL PROTECTED]> wrote:
I'm concerned about a couple things ...
1. You say you're not happy with the changes in PersistenceSessionFilter
which sounds ominous and to be honest I'm not even sure why you would
want to put any code in there. I much prefer the idea of putting the
code which checks if the application has been properly bootstrapped into
it's own filter. Call it the the BootstrapCheckFilter if you want. In
any case, i don't consider that logic to have anything to do with the
persistence session filter.
I put it in PersistenceSessionFilter because it was easy and related,
but I agree putting it in it's own filter is better. What I'm not so
happy about is the way I'm forwarding to the Struts actions.
how are you forwarding to the struts actions? do you mean code wise or
just logically?
Also, in my mind this filter should be very simple, such as ...
if(RollerFactory.isBootstrapped()) {
chain.doFilter(req, res);
} else {
// app not bootstrapped yet, prompt for user involvement
response.sendRedirect(install/upgrade url);
}
The logic is simple, but it is not as you describe. Currently, it is this:
* If DB connection fails forward to DatabaseError action
* Else if Roller tables not found direct to CreateDatabase action
* Else if Roller tables need upgrade direct to UpgradeDatabase action
I like your idea of having one central install action instead of
routing logic in the filter.
Right, my thinking is that it would just be easier and nicer for users
if this all happens in one connected process.
2. I don't know what you actually put in your SQLScriptRunner,
DatabaseCreator, and DatabaseUpgrader classes but I would have thought
this code could be part of the DatabaseProvider. It would be nice to
have more details on how these classes work.
Then I should commit them.
3. Why would we need users to restart or redeploy after doing the db
table work? In my mind that kind of defeats the purpose of making the
installation easy and really there should be no need for this.
I wouldn't say it defeats the purpose, but yes, it is an additional
step that should not be necessary if we have proper bootstrapping in
place.
I definitely think we should find a way to make this work without
forcing a restart.
4. I'm not sure exactly how your page flow is working. You have 3 new
actions defined but I don't understand how users get to each of them and
what happens after they take the action from each one.
There really is no page flow, if you don't have tables you get
directed to the CreateDatabase page, press the button, see results and
that's it.
I think that grouping these things together makes more sense.
Basically the way the setup page works. It lists the things that need
to be done, provides indication of which of them has been done, and
gives you a way to take the next step.
This brings up another question, which is why wouldn't this be part of
the setup process? I mean, our current setup page is effectively part
of properly installing the app, the only difference is that the setup
stuff happens after bootstrapping, but the workflow should be integrated.
My expectation is that there would be more of a unified experience such
that if the application has not been bootstrapped yet then the bootstrap
checking filter forwards all requests to a url such as
/roller-ui/bootstrap.rol to handle the bootstrapping process. From that
url you can walk through all parts of the process as needed i.e.
1. check that db connection works
2. do db table install/upgrade work
3. check that mail session works
4. everything is set, try bootstrapping application again
And these steps would basically be looped until the bootstrapping
process actually works. what i see so far doesn't seem to be as much of
a unified experience, but maybe i'm not understanding the flow?
I like that, but I have not gone that far yet, in part because I've
been separating installation and boostrapping in my mind and in my
workspaces. One reason I'd like to commit is so I can merge changes
into my roller_guice branch to experiment with DI based boostrapping
options.
I agree that they are separate, but in my mind I just consider
installation/upgrading as a required precursor to bootstrapping, so it
makes sense that the logic is if(!bootstrapped) { send user to
install/upgrade action }
-- Allen
I think I have a workable solution that is good enough to commit now
for more feedback and improvements. Though I'd like to move the code
from PersistenceSessionFilter first.
I do think that all the pages that you are showing there look great
though and I definitely think they will be a huge improvement over our
current installation process.
Yep. The database error pages alone should reduce install problem list
traffic significantly.
- Dave
