Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move settings from site.conf into webwork2.mojolicious.yml #2314

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

pstaabp
Copy link
Member

@pstaabp pstaabp commented Feb 6, 2024

This replaces the conf/site.conf file by adding many configuration variables to conf/webwork2.mojolicious.yml.

The idea is that many of the server configurations shouldn't be overridden by course configuration in site.conf or simple.conf. How this happens is that the new order of configurations reads is:

  1. webwork2.mojolicious.yml is loaded and some basic configuration is done.
  2. defaults.config
  3. localOverrides.conf
  4. simple.conf
  5. course.conf
  6. Configuration variables that shouldn't be overriden are then copied from webwork2.mojolicious.yml to CourseEnvironment.

Note that the URL and directory structure is now included in webwork2.mojolicious.yml so that has been removed from defaults.config.

Since yaml files cannot contain code, the loading of webwork2.mojolicious.yml is not done in the safe compartment.

Also, the code in conf/database.conf.dist has been moved to lib/WeBWorK/DB/Utils.pm -- This can be moved elsewhere if there is a better place for it. This was removed because it doesn't appear that there are the options that we used to allow in earlier versions.

Another strange thing that I noticed that when the session times out, I couldn't log back in. The addition of including the password on line 362 of lib/WeBWorK/Authen.pm fixed this, but I can't figure out how this work has changed that, but also, why the password was missing from that block.

Some other ideas I've thought about:

  • If this bloats webwork2.mojolicious.yml, we can move much of this to a different yaml file.
  • Move other settings from defaults.config to yaml files that wouldn't need to be loaded in the Safe
  • Move some of the items in the CourseEnvironment into the Mojolicious Controller config object or other helpers. Note: @drgrice1 suggested this, but will require rewriting a bunch of the codebase for a new location.

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

This is just a quick perusal of a few things, but I already see that there is a lot of work that needs to be done here. So much in fact, that I recommend not pursuing it further at this point. At least not in its entirety all at once.

How about you start with the removal of conf/database.conf.dist. Separate that into another pull request, and we will consider that. Then we will see what else here we want to pursue further after that.

One huge drawback of doing this is that those that have been using webwork will need to do a lot of work to transfer all of there site.conf settings to the webwork2.mojolicious.conf file. This is perhaps to much work when there may be only a few more releases of webwork2 before we start trying to switch to webwork3.

in `site.conf` as needed. In particular you will need to set `$server_root_url` to the server name, and set
- Copy `webwork2.mojolicious.dist.yml` to `webwork2.mojolicious.yml` and change appropriate settings.
- Copy `localOverrides.conf.dist` to `localOverrides.conf`, and adjust the variables
as needed. In particular you will need to set `$server_root_url` to the server name, and set
Copy link
Member

Choose a reason for hiding this comment

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

All of these references to variables that were in site.conf need to be updated. They are no longer perl variables, and so the $ should be removed.

@@ -14,7 +14,7 @@
#ScriptAliasMatch /webwork2_course_files/([^/]*)/show-source.cgi/(.*) /opt/webwork/courses/$1/html/show-source.cgi/$2
#ProxyPassMatch /webwork2_course_files/([^/]*)/show-source.cgi/(.*) !

# Note that if $webwork_url in site.conf is changed, then /webwork2
# Note that if $webwork_url in webwork2.mojolicious.yml is changed, then /webwork2
Copy link
Member

Choose a reason for hiding this comment

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

In this file as well, remove the $ on things reference in webwork2.mojolicious.yml. This will take some work, because this needs to be done anywhere a variable that was in site.conf and is now in webwork2.mojolicious.yml is referenced anywhere in the code or documentation files.

@@ -1,4 +1,4 @@
# Note that if $webwork_url in site.conf is changed, then /webwork2
# Note that if $webwork_url in webwork2.mojolicious.yml is changed, then /webwork2
Copy link
Member

Choose a reason for hiding this comment

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

In this file as well, remove the $ on things reference in webwork2.mojolicious.yml.

in `site.conf` as needed. In particular you will need to set `$server_root_url` to the server name, and set
- Copy `webwork2.mojolicious.dist.yml` to `webwork2.mojolicious.yml` and change appropriate settings.
- Copy `localOverrides.conf.dist` to `localOverrides.conf`, and adjust the variables
as needed. In particular you will need to set `$server_root_url` to the server name, and set
`$database_password` to the password for the database.
- Adjust the variables in `localOverrides.conf` to customize your server for your needs.
- Copy any of the other `.dist` files and adjust the variables in them as needed. Note that those files will need to be
Copy link
Member

Choose a reason for hiding this comment

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

There are comments about site.conf further down in this file that need to be updated.

Comment on lines +213 to +215
# Set these variables to correspond to your configuration. It is not
# recommended to change any of the settings in this file once your
# web server has been initially configured.
Copy link
Member

Choose a reason for hiding this comment

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

Remove the recommendation. That is a bad recommendation. If server needs change, then the configuration will need to be changed. I know you copied it from site.conf, but don't just copy the comments. Go through them and clean them up. There was a lot of crap said in site.conf.

Comment on lines +217 to +218
# URL of WeBWorK handler. If WeBWorK is to be on the web server root, use "". Note
# that using "" may not work so we suggest sticking with "/webwork2".
Copy link
Member

Choose a reason for hiding this comment

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

Clearly the comments about quoting do not apply anymore. Another reason to go through the comments.

@pstaabp
Copy link
Member Author

pstaabp commented Feb 20, 2024

Let's chat about this for our next meeting.

@Alex-Jordan
Copy link
Contributor

Note conflicts here now from a recent merge.

@pstaabp
Copy link
Member Author

pstaabp commented Mar 23, 2024

I'm going to close this in favor of #2338

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants