Skip to content

Commit

Permalink
Fix to restart Nginx once
Browse files Browse the repository at this point in the history
  • Loading branch information
@iliajie
iliajie committed Feb 4, 2024
1 parent b4a5af3 commit 94b6f1d
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions virtual_feature.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3438,7 +3438,11 @@ sub feature_add_protected_dir
};
&save_directive($server, [ ], [ $protected ]);
&flush_config_file_lines();
&apply_nginx();
&virtual_server::push_all_print();
&virtual_server::set_all_null_print();
&virtual_server::register_post_action(\&print_apply_nginx);
&virtual_server::run_post_actions();
&virtual_server::pop_all_print();
$status = 0;
}
&unlock_all_config_files();
Expand Down Expand Up @@ -3475,7 +3479,11 @@ sub feature_delete_protected_dir
# Can delete the location block
&save_directive($server, [ $loc ], [ ]);
&flush_config_file_lines();
&apply_nginx();
&virtual_server::push_all_print();
&virtual_server::set_all_null_print();
&virtual_server::register_post_action(\&print_apply_nginx);
&virtual_server::run_post_actions();
&virtual_server::pop_all_print();
$status = 0;
}
else {
Expand Down

4 comments on commit 94b6f1d

@iliajie
Copy link
Contributor Author

@iliajie iliajie commented on 94b6f1d Feb 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jamie, as you mentioned here, I also changed the yesterday's commit. It didn't work yesterday when I was trying it by just using:

&virtual_server::register_post_action(\&print_apply_nginx);

.. so I ended up using &apply_nginx();, which worked just fine. I'm surprised you didn't mention it at the first place?

So, now I ran more tests and using:

&virtual_server::push_all_print();
&virtual_server::set_all_null_print();
&virtual_server::register_post_action(\&print_apply_nginx);
&virtual_server::run_post_actions();
&virtual_server::pop_all_print();

.. works just fine. So, why using set_all_null_print is necessary in this case?

@jcameron
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't want the apply command to print anything (like "restarting Nginx"), use &apply_nginx instead of &print_apply_nginx. Then you don't need these lines :

&virtual_server::push_all_print();
&virtual_server::set_all_null_print();
...
&virtual_server::run_post_actions();
&virtual_server::pop_all_print();

@iliajie
Copy link
Contributor Author

@iliajie iliajie commented on 94b6f1d Feb 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then the restart will happen multiple times. What if 20 protected directories deleted at once?

We don't want that, right?

@jcameron
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly why to use register_post_action, because it guarantees that each action is only run once at the end of the script.

Please sign in to comment.