os-autoinst / openQA

openQA web-frontend, scheduler and tools.
http://openqa.opensuse.org/
GNU General Public License v2.0
314 stars 206 forks source link

Allow configure-web-proxy to use a custom port #5769

Closed kalikiana closed 1 month ago

kalikiana commented 1 month ago

See: https://progress.opensuse.org/issues/162611

perlpunk commented 1 month ago

I don't understand the usage anymore right now. I checked out your branch and ran some tests:


# bad
% ./script/configure-web-proxy  -P 8080
$web_proxy=apache $web_port=--
% ../script/configure-web-proxy  -p nginx -P 8080
$web_proxy=nginx $web_port=--
# good
% ../script/configure-web-proxy  --port 8080
$web_proxy=apache $web_port=8080
% ../script/configure-web-proxy  -p nginx --port 8080
$web_proxy=nginx $web_port=8080
kalikiana commented 1 month ago

I would prefer if the script left nginx.conf/listen.conf untouched if no port is specified.

That would lead to surprises when running the script more than once unless we start parsing the original file.

perlpunk commented 1 month ago

I would prefer if the script left nginx.conf/listen.conf untouched if no port is specified.

That would lead to surprises when running the script more than once unless we start parsing the original file.

I would agree with @Martchus, leave it untouched if there was no web_port parameter given. I don't see why it's problematic.

github-actions[bot] commented 1 month ago

Great PR! Please pay attention to the following items before merging:

Files matching docs/*.asciidoc:

This is an automatically generated QA checklist based on modified files.