Bug 1455495 - Replace apache with Mojolicious by dylanwh · Pull Request #517 · mozilla-bteam/bmo

dylanwh · 2018-04-04T03:06:05Z

I gave a talk about this, and the slides are available here:
https://dylanhardison.files.wordpress.com/2018/09/bugzilla-on-mojolicious.pdF

mohawk2 · 2018-04-04T04:13:52Z

This looks great to me. How about some fingers-and-toes tests too?

dylanwh · 2018-04-05T01:25:36Z

@mohawk2 not sure I understand fingers and toes tests?

mohawk2 · 2018-04-05T16:17:14Z

@dylanwh Sorry to be unclear. The analogy is the most basic of testing, to "count the fingers and toes" are the correct number. If you tell me the /new/ already has tests that ensure correct behaviour, that's good enough for me :-)

I see the CI test_sanity is currently failing? t/002goodperl.t fails test 66.

mohawk2 · 2018-05-19T16:22:37Z

Uh oh, still failing CI :-(

dylanwh · 2018-07-19T16:28:33Z

Paging @mohawk2, your review of Bugzilla/Quantum/CGI.pm's _STDIN() method and related code would be valuable. :-)

dylanwh · 2018-07-19T16:36:42Z

+    );
+    $ses_auth->any('/index.cgi')->to('SES#main');
+
+    $r->any('/:REWRITE_itrequest' => [REWRITE_itrequest => qr{form[\.:]itrequest}])->to(


Perhaps I should add a hook and move these to the BMO extension?
@globau ?

i agree; these belong in the bmo ext. this needn't block the initial release however.

globau · 2018-07-23T14:41:15Z

+    );
+    $ses_auth->any('/index.cgi')->to('SES#main');
+
+    $r->any('/:REWRITE_itrequest' => [REWRITE_itrequest => qr{form[\.:]itrequest}])->to(


i agree; these belong in the bmo ext. this needn't block the initial release however.

globau · 2018-07-23T15:16:24Z

+        SCRIPT_NAME     => "$prefix$script_name",
+        SERVER_NAME     => hostname,
+        SERVER_PORT     => $tx->local_port,
+        SERVER_PROTOCOL => $req->is_secure ? 'HTTPS' : 'HTTP', # TODO: Version is missing


globau · 2018-07-23T15:18:18Z

+sub _file_to_method {
+    my ($name) = @_;
+    $name =~ s/\./_/s;
+    $name =~ s/\W+/_/gs;


no need to replace periods separately, \W will match

globau · 2018-07-23T15:38:00Z

+        return $self->template->process($file, $vars, $output);
+    }
+    else {
+        die __PACKAGE__ . '->process() called with too many arguments';


it's a bit odd to see the length of @_ being used to see if vars are set.

i suspect this check is useful only during the development of the mojo transition; it would be much clearer if you just did if (!defined $output) { ... } else { ... }

globau · 2018-07-23T16:05:28Z

    return $self->SUPER::redirect(@_);
 }

+use Bugzilla::Logging;


this should be at the top of the file

globau · 2018-07-25T06:57:14Z

+  #         && $ENV{'HTTP_USER_AGENT'} !~ /(?:WebKit|Trident|KHTML)/
+  #           && !$agent
+  #             && !defined($cgi->param('serverpush'))
+  #               || $cgi->param('serverpush');


is serverpush a TODO item, or is it being dropped by this change?
if it's being dropped you should remove all related code.

globau · 2018-07-25T06:58:53Z

+      ->to( 'CGI#enter_bug_cgi' => { 'product' => 'Shield', 'format' => 'shield-studies' } );
+    $r->any( '/:REWRITE_client_bounty' => [ REWRITE_client_bounty => qr{form[\.:]client[\.:]bounty} ] )
+      ->to( 'CGI#enter_bug_cgi' => { 'product' => 'Firefox', 'format' => 'client-bounty' } );
+}


this is very hard to read, and repetitive.

how about defining these aliases in a hash, and building the routes dynamically from that?

globau · 2018-07-25T07:00:25Z

@@ -1,72 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public


what's happing here?

if the shutdownhtml is no longer supported then all the related code should be removed.
if it is still supported then this test should remain.

globau · 2018-07-25T07:01:15Z


 sub cmd_test_selenium {
    my $conf = require $ENV{BZ_QA_CONF_FILE};
+    $ENV{HTTP_BACKEND} = 'simple';


perhaps it would be better to run tests under the same backend as prod?

The selenium tests don't run with any level of concurrency and haven't for a long time.
For apache the number of workers as also limited. I'm confident hypnotoad and the simple httpd are close enough for this.

globau · 2018-07-25T07:02:45Z

+#
+#
+#
+### END INIT INFO


i've been using systemd for so long i forgot how horrible init.d scripts are

I believe changes have been addressed.

edmorley · 2018-09-12T08:50:21Z

Hi! Bug 1355476 was duped to bug 1455495, however I don't see any reference to immutable in this PR? Is that going to be added later? :-)

dylanwh added the WIP label Apr 4, 2018

dylanwh changed the title ~~initial proof of concept of mojolicious inside bmo~~ Embed Mojolicious inside BMO Apr 20, 2018

dylanwh changed the title ~~Embed Mojolicious inside BMO~~ Replace Apache with Mojolicious May 21, 2018

dylanwh changed the title ~~Replace Apache with Mojolicious~~ Bug 1455495 - Replace apache with Mojolicious Jun 29, 2018

dylanwh changed the base branch from master to pre-mojo July 18, 2018 20:33

dylanwh removed the WIP label Jul 19, 2018

dylanwh requested review from globau, purelogiq and smacleod July 19, 2018 16:20

dylanwh commented Jul 19, 2018

View reviewed changes

globau previously requested changes Jul 25, 2018

View reviewed changes

dylanwh changed the base branch from pre-mojo to master July 31, 2018 17:06

dylanwh added 11 commits September 13, 2018 13:13

Bug 1455495 - Replace apache with Mojolicious

db698d4

fix lint errors

dbf9c53

fix errors from group review

7372988

some cleanups

1ce7117

more tidy

150224a

more tidy

7c521ed

more tidy

31b5a98

review nits

68f7b25

remove unused HTTPD_* envs from everywhere

9896239

hypnotoad should always expect a proxy

dfee08b

Add EV

8abf2b6

dylanwh and others added 20 commits September 13, 2018 13:13

with EV

801de20

add version

d6a505b

add more debugging but also probably fix redirect problem

433dee9

Fix a module loading problem

7bca649

handle bare bugs

7e2bedc

fix route

1c7f515

no bug - Fixed routing issues with /bug/<bug_id> and /<bug_id>

3e135d2

fix glaring error

a8e5b5e

add more testing

9daf493

better FATAL error

f4009f2

diagnostic code is a string

66f0519

one more slurpy type

d6f64e1

once more

1186906

more type checking

defd60b

update deps

722af2c

remove more apache bits

9d6b9de

add base image

8c2120a

add favicon

7a39f67

add graphs path

afe214c

add /bzapi path

bb3130a

dylanwh merged commit b8b2a94 into mozilla-bteam:master Sep 18, 2018

dylanwh deleted the mojo-poc branch September 18, 2018 22:19

kyoshino mentioned this pull request Oct 7, 2018

Bug 1497070 - In-page links are broken due to <base href> added during Mojo migration #812

Merged

		@@ -1,72 +0,0 @@
		# This Source Code Form is subject to the terms of the Mozilla Public

Conversation

dylanwh commented Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohawk2 commented Apr 4, 2018

Uh oh!

dylanwh commented Apr 5, 2018

Uh oh!

mohawk2 commented Apr 5, 2018

Uh oh!

mohawk2 commented May 19, 2018

Uh oh!

dylanwh commented Jul 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edmorley commented Sep 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

dylanwh commented Apr 4, 2018 •

edited

Loading