Skip to content

Remove WeBWorK::Constants and improve the webwork2 app configuration. #1985

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

Merged

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented May 13, 2023

The only constants still defined in the WeBWorK::Constants package were the WEBWORK_DIRECTORY and the PG_DIRECTORY. Furthermore, even those were not used much in the code, and they are even set several times in scripts to "avoid an error message" from not having them defined. The few places that these constants were used now either use WEBWORK_ROOT and PG_ROOT environment variables directly (from which the previous constants were set) or use values from the course environment.

The CourseEnvironment constructor grabs webwork_dir and pg_dir from the %WeBWorK::SeedCE hash if those parameters are not passed in, and falls back as a last resort to WEBWORK_ROOT and PG_ROOT environment variables. This makes it so that within the webwork app, you do not need to pass the webwork_dir to construct a CourseEnvironment object.

Scripts will either need to set the %WeBWorK::SeedCE webwork_dir and pg_dir values, or pass webwork_dir and pg_dir to the CourseEnvironment constructor in its now optional argument, or just ensure the WEBWORK_ROOT and PG_ROOT environment variables are set.

Note the old four argument way of calling the CourseEnvironment constructor that has been marked as deprecated since 2004 is now removed entirely.

One upshot of this is that the pg_dir is not needed in site.conf (and so is removed), and this now only needs to be set in one place, namely webwork2.mojolicious.yml.

The other constants defined throughout the code that were previously set in WeBWorK::Constants are now set from configuration variables in webwork2.mojolicious.yml when the app initializes. The only such constants are the WeBWorK::Debug constants and one WeBWorK::Hardcopy constant. This means that to enable debug logging, you don't need to modify the code anymore. Just enable it in the webwork2.mojolicious.yml file.

Note the clients and t directories were also deleted. I am tired of seeing matches on things in these directories when grepping the code, and nothing in these directories works or is valid for anything anymore.

@drgrice1 drgrice1 force-pushed the course-environment-constants-rework branch 2 times, most recently from df1d695 to f3d143a Compare May 15, 2023 12:17
Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

For testing, I just tried to run through all of the pages and looks good. I would argue that let's merge this soon and if anything was missed we'll see with just general use.

@drdrew42
Copy link
Member

This will have implications for the standalone renderer... it will be nice to ditch the WeBWorK::Constants!

@drgrice1
Copy link
Member Author

Technically since the standalone renderer has its own code for this, it will not need any changes due to this pull request. However, it would be good to make similar changes to the standalone renderer's code.

@drgrice1 drgrice1 force-pushed the course-environment-constants-rework branch 2 times, most recently from e907ca6 to 094bef6 Compare May 15, 2023 21:24
drgrice1 added 2 commits May 15, 2023 16:26
The only constants still defined in the WeBWorK::Constants package were
the WEBWORK_DIRECTORY and the PG_DIRECTORY.  Furthermore, even those
were not used much in the code, and they are even set several times in
scripts to "avoid an error message" from not having them defined.  The
few places that these constants were used now either use WEBWORK_ROOT
and PG_ROOT environment variables directory (from which the previous
constants were set) or use values from the course environment.

The CourseEnvironment constructor grabs webwork_dir and pg_dir from the
%WeBWorK::SeedCE hash if those parameters are not passed in, and falls
back as a last resort to WEBWORK_ROOT and PG_ROOT environment variables.
This makes it so that within the webwork app, you do not need to pass
the webwork_dir to construct a CourseEnvironment object.

Scripts will either need to set the %WeBWorK::SeedCE webwork_dir and
pg_dir values, or pass webwork_dir and pg_dir to the CourseEnvironment
constructor in its now optional argument, or just ensure the
WEBWORK_ROOT and PG_ROOT environment variables are set.

Note the old four argument way of calling the CourseEnvironment
constructor that has been marked as deprecated since 2004 is now removed
entirely.

One upshot of this is that the pg_dir is not needed in site.conf (and so
is removed), and this now only needs to be set in one place, namely
webwork2.mojolicious.yml.

The other constants defined throughout the code that were previously set
in WeBWorK::Constants are now set from configuration variables in
webwork2.mojolicious.yml when the app initializes.  The only such
constants are the WeBWorK::Debug constants and one WeBWorK::Hardcopy
constant.  This means that to enable debug logging, you don't need to
modify the code anymore.  Just enable it in the webwork2.mojolicious.yml
file.
I am tired of seeing matches on things in these directories when
grepping for variables that are obsolete, and nothing in these
directories works anymore.
@drgrice1 drgrice1 force-pushed the course-environment-constants-rework branch from 094bef6 to ba6f402 Compare May 15, 2023 21:26
@drgrice1
Copy link
Member Author

@drdrew42: I realized that I have already done this for the standalone renderer. It is part of openwebwork/renderer#7.

Copy link
Contributor

@somiaj somiaj left a comment

Choose a reason for hiding this comment

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

Tested pages out for a bit with this, everything seems to work.

@pstaabp pstaabp merged commit a88ee4a into openwebwork:WeBWorK-2.18 May 18, 2023
@drgrice1 drgrice1 deleted the course-environment-constants-rework branch May 18, 2023 19:55
drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request May 19, 2023
This was using the old four argument form of the CourseEnvironment
constructor.  I missed that in openwebwork#1985.

To test this rename a course or unarchive a course to a new name.  On
the current release candidate branch you will get an error.  With this
pull request it will work.

Note that if you test this on the release candidate branch and rename a
course, the course directory will be renamed.  So you will need to
manually rename that back to what it was.  The database tables will
still be under the old course name.  If you attempt to unarchive to a
new name, then the course directory will be created with the new name,
but the database tables will not be.  So you will need to manually
delete that new course directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants