Convert to PSGI #4

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants
@berekuk

berekuk commented Jun 3, 2012

Ok, it's basically working.

Things I checked:

  • channel list
  • channel index
  • log
  • ?admin=1 mode and spam
  • search by nick

Couldn't check search by text - I hacked ilbot to run on sqlite before I went for PSGI port, and it's basically working, but search by text relies on mysql dialect. I intend to send sqlite patch separately.

Main app is ilbot.psgi. I used manual routing there.
Didn't want to use Dancer or any other framework, and Plack::Middleware::Rewrite didn't work for me - see ap/Plack-Middleware-Rewrite#2.
So routing part is a bit messy, but I think it's still improvement comparing to mod_rewrite :)

I did support backward compatibility for /out.pl, since it was supported in old .htaccess file.

One important thing I didn't test is whether it's working when app's base url is not /.

Oh, and tests are failing. Will get to fixing them now, don't merge this yet :)

@berekuk

This comment has been minimized.

Show comment Hide comment
@berekuk

berekuk Jun 3, 2012

Ok, failing tests are not my fault. t/decode.t and t/revision-links.t are broken my my machine. (perl 5.16, MacOSX 10.7)

berekuk commented Jun 3, 2012

Ok, failing tests are not my fault. t/decode.t and t/revision-links.t are broken my my machine. (perl 5.16, MacOSX 10.7)

@stephan48

This comment has been minimized.

Show comment Hide comment
@stephan48

stephan48 Jun 4, 2012

Collaborator

Cool, about the sqlite thing we should think about an db abstraction lager, Moritz uses mysql and i use pgsql. I will talk about it with Moritz...

Thanks for your time for porting the app to psgi, I think we should check the function when not mounted under root.

Vyacheslav Matyukhin reply@reply.github.com schrieb:

Ok, it's basically working.

Things I checked:

  • channel list
  • channel index
  • log
  • ?admin=1 mode and spam
  • search by nick

Couldn't check search by text - I hacked ilbot to run on sqlite before I went for PSGI port, and it's basically working, but search by text relies on mysql dialect. I intend to send sqlite patch separately.

Main app is ilbot.psgi. I used manual routing there.
Didn't want to use Dancer or any other framework, and Plack::Middleware::Rewrite didn't work for me - see ap/Plack-Middleware-Rewrite#2.
So routing part is a bit messy, but I think it's still improvement comparing to mod_rewrite :)

I did support backward compatibility for /out.pl, since it was supported in old .htaccess file.

One important thing I didn't test is whether it's working when app's base url is not /.

Oh, and tests are failing. Will get to fixing them now, don't merge this yet :)

You can merge this Pull Request by running:

git pull https://github.com/berekuk/ilbot psgi

Or you can view, comment on it, or merge it online at:

#4

-- Commit Summary --

  • Convert to PSGI
  • routing fixes

-- File Changes --

M README (2)
D cgi/search.pl (201)
D cgi/spam.pl (64)
D cgi/text.pl (77)
A ilbot.psgi (60)
M lib/IrcLog/WWW.pm (11)
A www/.htaccess (0)
A www/abbr.dat (0)
R www/channel-index.pl (57)
A www/channels/crimsonfu.tmpl (0)
A www/channels/parrot.tmpl (0)
A www/channels/perl6.tmpl (0)
A www/database.conf (1)
R www/index.pl (46)
A www/links.dat (0)
R www/out.pl (205)
A www/search.pl (204)
A www/spam.pl (70)
A www/static/at.png (0)
A www/static/camelia.png (0)
A www/static/jquery.min.js (0)
A www/static/moose1.ico (0)
A www/static/moosecamel.png (0)
A www/static/nickfilter.js (0)
A www/static/style.css (0)
A www/template/channel-index.tmpl (0)
A www/template/day.tmpl (0)
R www/template/footer.tmpl (2)
A www/template/index.tmpl (0)
A www/template/line.tmpl (0)
A www/template/linkblock.tmpl (0)
A www/template/search.tmpl (0)
R www/template/spam.tmpl (2)
A www/text.pl (84)
A www/www.conf (0)

-- Patch Links --

https://github.com/moritz/ilbot/pull/4.patch
https://github.com/moritz/ilbot/pull/4.diff


Reply to this email directly or view it on GitHub:
#4

Collaborator

stephan48 commented Jun 4, 2012

Cool, about the sqlite thing we should think about an db abstraction lager, Moritz uses mysql and i use pgsql. I will talk about it with Moritz...

Thanks for your time for porting the app to psgi, I think we should check the function when not mounted under root.

Vyacheslav Matyukhin reply@reply.github.com schrieb:

Ok, it's basically working.

Things I checked:

  • channel list
  • channel index
  • log
  • ?admin=1 mode and spam
  • search by nick

Couldn't check search by text - I hacked ilbot to run on sqlite before I went for PSGI port, and it's basically working, but search by text relies on mysql dialect. I intend to send sqlite patch separately.

Main app is ilbot.psgi. I used manual routing there.
Didn't want to use Dancer or any other framework, and Plack::Middleware::Rewrite didn't work for me - see ap/Plack-Middleware-Rewrite#2.
So routing part is a bit messy, but I think it's still improvement comparing to mod_rewrite :)

I did support backward compatibility for /out.pl, since it was supported in old .htaccess file.

One important thing I didn't test is whether it's working when app's base url is not /.

Oh, and tests are failing. Will get to fixing them now, don't merge this yet :)

You can merge this Pull Request by running:

git pull https://github.com/berekuk/ilbot psgi

Or you can view, comment on it, or merge it online at:

#4

-- Commit Summary --

  • Convert to PSGI
  • routing fixes

-- File Changes --

M README (2)
D cgi/search.pl (201)
D cgi/spam.pl (64)
D cgi/text.pl (77)
A ilbot.psgi (60)
M lib/IrcLog/WWW.pm (11)
A www/.htaccess (0)
A www/abbr.dat (0)
R www/channel-index.pl (57)
A www/channels/crimsonfu.tmpl (0)
A www/channels/parrot.tmpl (0)
A www/channels/perl6.tmpl (0)
A www/database.conf (1)
R www/index.pl (46)
A www/links.dat (0)
R www/out.pl (205)
A www/search.pl (204)
A www/spam.pl (70)
A www/static/at.png (0)
A www/static/camelia.png (0)
A www/static/jquery.min.js (0)
A www/static/moose1.ico (0)
A www/static/moosecamel.png (0)
A www/static/nickfilter.js (0)
A www/static/style.css (0)
A www/template/channel-index.tmpl (0)
A www/template/day.tmpl (0)
R www/template/footer.tmpl (2)
A www/template/index.tmpl (0)
A www/template/line.tmpl (0)
A www/template/linkblock.tmpl (0)
A www/template/search.tmpl (0)
R www/template/spam.tmpl (2)
A www/text.pl (84)
A www/www.conf (0)

-- Patch Links --

https://github.com/moritz/ilbot/pull/4.patch
https://github.com/moritz/ilbot/pull/4.diff


Reply to this email directly or view it on GitHub:
#4

@berekuk

This comment has been minimized.

Show comment Hide comment
@berekuk

berekuk Jun 4, 2012

We also should think about adding authorization for /spam.pl by default (I think there must be a middleware for that?).
Current installation instructions say:

Be sure to password-protect the file spam.pl with a .htaccess file.

berekuk commented Jun 4, 2012

We also should think about adding authorization for /spam.pl by default (I think there must be a middleware for that?).
Current installation instructions say:

Be sure to password-protect the file spam.pl with a .htaccess file.

@stephan48

This comment has been minimized.

Show comment Hide comment
@stephan48

stephan48 Jun 4, 2012

Collaborator

Am 04.06.2012 12:30, schrieb Vyacheslav Matyukhin:

We also should think about adding authorization for /spam.pl by
default (I think there must be a middleware for that?).
Current installation instructions say:

Be sure to password-protect the file spam.pl with a .htaccess file.


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

I use password protection for the whole webapp, and i got special
checks in place for checking who is allowed to see which channels, and
in this part i also got checks in place for who is allowed to do spam
marking.

Also i got a fix in place when admin mode is enabled so that it isnt
saved in the template cache.
Which does the current code, i will try to commit them back as soon as
possible :)

Collaborator

stephan48 commented Jun 4, 2012

Am 04.06.2012 12:30, schrieb Vyacheslav Matyukhin:

We also should think about adding authorization for /spam.pl by
default (I think there must be a middleware for that?).
Current installation instructions say:

Be sure to password-protect the file spam.pl with a .htaccess file.


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

I use password protection for the whole webapp, and i got special
checks in place for checking who is allowed to see which channels, and
in this part i also got checks in place for who is allowed to do spam
marking.

Also i got a fix in place when admin mode is enabled so that it isnt
saved in the template cache.
Which does the current code, i will try to commit them back as soon as
possible :)

@berekuk

This comment has been minimized.

Show comment Hide comment
@berekuk

berekuk Jun 30, 2012

I finally got to testing non-root mounts. Fixed one redirect issue, but otherwise it's all good.

You have to manually edit BASE_URL in www/www.conf, but the same option was in the old cgi/cgi.conf file. I guess it could be automated after some additional refactorings, but I think we need to merge this branch first.

berekuk commented Jun 30, 2012

I finally got to testing non-root mounts. Fixed one redirect issue, but otherwise it's all good.

You have to manually edit BASE_URL in www/www.conf, but the same option was in the old cgi/cgi.conf file. I guess it could be automated after some additional refactorings, but I think we need to merge this branch first.

@berekuk

This comment has been minimized.

Show comment Hide comment
@berekuk

berekuk Oct 2, 2012

@moritz, are you still going to apply this?
I just rebased this branch against the current master and converted save_summary.pl to PSGI too, but it's becoming hard to maintain this fork, since so much code got moved around.

berekuk commented Oct 2, 2012

@moritz, are you still going to apply this?
I just rebased this branch against the current master and converted save_summary.pl to PSGI too, but it's becoming hard to maintain this fork, since so much code got moved around.

@moritz

This comment has been minimized.

Show comment Hide comment
@moritz

moritz Oct 3, 2012

Owner

On 10/03/2012 01:17 AM, Vyacheslav Matyukhin wrote:

@moritz https://github.com/moritz, are you still going to apply this?
I just rebased this branch against the current master and converted
save_summary.pl to PSGI too, but it's becoming hard to maintain this
fork, since so much code got moved around.

Sorry, I still haven't looked closely at it, and thus haven't decided
yet. I hope I'll get to it this week.

Cheers,
Moritz

Owner

moritz commented Oct 3, 2012

On 10/03/2012 01:17 AM, Vyacheslav Matyukhin wrote:

@moritz https://github.com/moritz, are you still going to apply this?
I just rebased this branch against the current master and converted
save_summary.pl to PSGI too, but it's becoming hard to maintain this
fork, since so much code got moved around.

Sorry, I still haven't looked closely at it, and thus haven't decided
yet. I hope I'll get to it this week.

Cheers,
Moritz

@berekuk

This comment has been minimized.

Show comment Hide comment
@berekuk

berekuk Feb 26, 2013

Ok, this is my final attempt. To merge or not to merge? :)

We're moving from IRC to using a Jabber conference for our team, so I probably won't support our ilbot instance for much longer...

berekuk commented Feb 26, 2013

Ok, this is my final attempt. To merge or not to merge? :)

We're moving from IRC to using a Jabber conference for our team, so I probably won't support our ilbot instance for much longer...

@moritz

This comment has been minimized.

Show comment Hide comment
@moritz

moritz May 11, 2013

Owner

After having a few more ideas about the direction that ilbot is going, I've decided to a pretty big refactoring in the 'nextgen' branch. That features a saner URL layout, saner directory layout, and also a PSGI backend.

So I'm closing your pull request now. Please accept my apologies for letting you wait so long, and then not accepting the patch after all; I really feel bad about it.

Owner

moritz commented May 11, 2013

After having a few more ideas about the direction that ilbot is going, I've decided to a pretty big refactoring in the 'nextgen' branch. That features a saner URL layout, saner directory layout, and also a PSGI backend.

So I'm closing your pull request now. Please accept my apologies for letting you wait so long, and then not accepting the patch after all; I really feel bad about it.

@moritz moritz closed this May 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment